From 199dcffe174d68ffe9984ab471565084b95788eb Mon Sep 17 00:00:00 2001 From: Xin Rong Date: Tue, 14 Feb 2023 10:39:45 +0800 Subject: [PATCH] feat: support disable status (#1595) --- cmd/ingress/ingress.go | 1 + conf/config-default.yaml | 6 +- pkg/config/config.go | 36 +-- pkg/config/config_test.go | 31 ++- pkg/providers/apisix/apisix_cluster_config.go | 5 +- pkg/providers/apisix/apisix_consumer.go | 3 + pkg/providers/apisix/apisix_plugin_config.go | 5 +- pkg/providers/apisix/apisix_route.go | 16 +- pkg/providers/apisix/apisix_tls.go | 5 +- pkg/providers/apisix/apisix_upstream.go | 5 +- pkg/providers/controller.go | 1 + pkg/providers/gateway/gateway.go | 2 +- pkg/providers/gateway/provider.go | 2 + pkg/providers/ingress/ingress.go | 61 +++-- pkg/providers/utils/ingress_status.go | 104 ++++++- pkg/providers/utils/ingress_status_test.go | 257 ++++++++++++++++++ pkg/providers/utils/status.go | 16 ++ pkg/providers/utils/status_test.go | 102 +++++++ test/e2e/scaffold/ingress.go | 41 ++- test/e2e/scaffold/scaffold.go | 1 + .../suite-ingress-features/status.go | 74 +++++ 21 files changed, 690 insertions(+), 84 deletions(-) create mode 100644 pkg/providers/utils/ingress_status_test.go create mode 100644 pkg/providers/utils/status_test.go diff --git a/cmd/ingress/ingress.go b/cmd/ingress/ingress.go index 2b2b9a6e7b..c38485f8f4 100644 --- a/cmd/ingress/ingress.go +++ b/cmd/ingress/ingress.go @@ -186,6 +186,7 @@ For example, no available LB exists in the bare metal environment.`) cmd.PersistentFlags().StringVar(&cfg.Kubernetes.APIVersion, "api-version", config.DefaultAPIVersion, config.APIVersionDescribe) cmd.PersistentFlags().BoolVar(&cfg.Kubernetes.WatchEndpointSlices, "watch-endpointslices", false, "whether to watch endpointslices rather than endpoints") cmd.PersistentFlags().BoolVar(&cfg.Kubernetes.EnableGatewayAPI, "enable-gateway-api", false, "whether to enable support for Gateway API") + cmd.PersistentFlags().BoolVar(&cfg.Kubernetes.DisableStatusUpdates, "disable-status-updates", false, "Disable resource status updates") cmd.PersistentFlags().StringVar(&cfg.APISIX.AdminAPIVersion, "apisix-admin-api-version", "v2", `the APISIX admin API version. can be "v2" or "v3". Default value is v2.`) cmd.PersistentFlags().StringVar(&cfg.APISIX.DefaultClusterBaseURL, "default-apisix-cluster-base-url", "", "the base URL of admin api / manager api for the default APISIX cluster") cmd.PersistentFlags().StringVar(&cfg.APISIX.DefaultClusterAdminKey, "default-apisix-cluster-admin-key", "", "admin key used for the authorization of admin api / manager api for the default APISIX cluster") diff --git a/conf/config-default.yaml b/conf/config-default.yaml index 7610ded4a1..9b3570a048 100644 --- a/conf/config-default.yaml +++ b/conf/config-default.yaml @@ -50,6 +50,7 @@ ingress_status_address: [] # when there is no available information on the Ser enable_profiling: true # enable profiling via web interfaces # host:port/debug/pprof, default is true. apisix-resource-sync-interval: "300s" # Default interval for synchronizing Kubernetes resources to APISIX + # Kubernetes related configurations. kubernetes: kubeconfig: "" # the Kubernetes configuration file path, default is @@ -82,11 +83,14 @@ kubernetes: api_version: apisix.apache.org/v2 # the default value of API version is "apisix.apache.org/v2", support "apisix.apache.org/v2beta3" and "apisix.apache.org/v2". plugin_metadata_cm: plugin-metadata-config-map + + disable_status_updates: false # In the case of a large number of resources and the status of resources is not concerned + # you can consider disabling status to speed up the synchronization cycle of resources. # APISIX related configurations. apisix: admin_api_version: v3 # the APISIX admin API version. can be "v2" or "v3" - default_cluster_base_url: "http://127.0.0.1:9080/apisix/admin" # The base url of admin api / manager api + default_cluster_base_url: "http://127.0.0.1:9180/apisix/admin" # The base url of admin api / manager api # of the default APISIX cluster default_cluster_admin_key: "" # the admin key used for the authentication of admin api / manager api in the diff --git a/pkg/config/config.go b/pkg/config/config.go index 98283535a1..2e7c63f21f 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -90,15 +90,16 @@ type Config struct { // KubernetesConfig contains all Kubernetes related config items. type KubernetesConfig struct { - Kubeconfig string `json:"kubeconfig" yaml:"kubeconfig"` - ResyncInterval types.TimeDuration `json:"resync_interval" yaml:"resync_interval"` - NamespaceSelector []string `json:"namespace_selector" yaml:"namespace_selector"` - ElectionID string `json:"election_id" yaml:"election_id"` - IngressClass string `json:"ingress_class" yaml:"ingress_class"` - IngressVersion string `json:"ingress_version" yaml:"ingress_version"` - WatchEndpointSlices bool `json:"watch_endpoint_slices" yaml:"watch_endpoint_slices"` - APIVersion string `json:"api_version" yaml:"api_version"` - EnableGatewayAPI bool `json:"enable_gateway_api" yaml:"enable_gateway_api"` + Kubeconfig string `json:"kubeconfig" yaml:"kubeconfig"` + ResyncInterval types.TimeDuration `json:"resync_interval" yaml:"resync_interval"` + NamespaceSelector []string `json:"namespace_selector" yaml:"namespace_selector"` + ElectionID string `json:"election_id" yaml:"election_id"` + IngressClass string `json:"ingress_class" yaml:"ingress_class"` + IngressVersion string `json:"ingress_version" yaml:"ingress_version"` + WatchEndpointSlices bool `json:"watch_endpoint_slices" yaml:"watch_endpoint_slices"` + APIVersion string `json:"api_version" yaml:"api_version"` + EnableGatewayAPI bool `json:"enable_gateway_api" yaml:"enable_gateway_api"` + DisableStatusUpdates bool `json:"disable_status_updates" yaml:"disable_status_updates"` } // APISIXConfig contains all APISIX related config items. @@ -133,14 +134,15 @@ func NewDefaultConfig() *Config { EnableProfiling: true, ApisixResourceSyncInterval: types.TimeDuration{Duration: 300 * time.Second}, Kubernetes: KubernetesConfig{ - Kubeconfig: "", // Use in-cluster configurations. - ResyncInterval: types.TimeDuration{Duration: 6 * time.Hour}, - ElectionID: IngressAPISIXLeader, - IngressClass: IngressClass, - IngressVersion: IngressNetworkingV1, - APIVersion: DefaultAPIVersion, - WatchEndpointSlices: false, - EnableGatewayAPI: false, + Kubeconfig: "", // Use in-cluster configurations. + ResyncInterval: types.TimeDuration{Duration: 6 * time.Hour}, + ElectionID: IngressAPISIXLeader, + IngressClass: IngressClass, + IngressVersion: IngressNetworkingV1, + APIVersion: DefaultAPIVersion, + WatchEndpointSlices: false, + EnableGatewayAPI: false, + DisableStatusUpdates: false, }, APISIX: APISIXConfig{ AdminAPIVersion: "v2", diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index dfb7dc1065..e931716ddd 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -42,12 +42,13 @@ func TestNewConfigFromFile(t *testing.T) { EnableProfiling: true, ApisixResourceSyncInterval: types.TimeDuration{Duration: 200 * time.Second}, Kubernetes: KubernetesConfig{ - ResyncInterval: types.TimeDuration{Duration: time.Hour}, - Kubeconfig: "/path/to/foo/baz", - ElectionID: "my-election-id", - IngressClass: IngressClass, - IngressVersion: IngressNetworkingV1, - APIVersion: DefaultAPIVersion, + ResyncInterval: types.TimeDuration{Duration: time.Hour}, + Kubeconfig: "/path/to/foo/baz", + ElectionID: "my-election-id", + IngressClass: IngressClass, + IngressVersion: IngressNetworkingV1, + APIVersion: DefaultAPIVersion, + DisableStatusUpdates: true, }, APISIX: APISIXConfig{ AdminAPIVersion: "v2", @@ -94,6 +95,7 @@ kubernetes: ingress_class: apisix ingress_version: networking/v1 api_version: apisix.apache.org/v2 + disable_status_updates: true apisix: admin_api_version: v2 default_cluster_base_url: http://127.0.0.1:8080/apisix @@ -132,12 +134,13 @@ func TestConfigWithEnvVar(t *testing.T) { EnableProfiling: true, ApisixResourceSyncInterval: types.TimeDuration{Duration: 200 * time.Second}, Kubernetes: KubernetesConfig{ - ResyncInterval: types.TimeDuration{Duration: time.Hour}, - Kubeconfig: "", - ElectionID: "my-election-id", - IngressClass: IngressClass, - IngressVersion: IngressNetworkingV1, - APIVersion: DefaultAPIVersion, + ResyncInterval: types.TimeDuration{Duration: time.Hour}, + Kubeconfig: "", + ElectionID: "my-election-id", + IngressClass: IngressClass, + IngressVersion: IngressNetworkingV1, + APIVersion: DefaultAPIVersion, + DisableStatusUpdates: true, }, APISIX: APISIXConfig{ AdminAPIVersion: "v2", @@ -173,7 +176,8 @@ func TestConfigWithEnvVar(t *testing.T) { "resync_interval": "1h0m0s", "election_id": "my-election-id", "ingress_class": "apisix", - "ingress_version": "networking/v1" + "ingress_version": "networking/v1", + "disable_status_updates": true }, "apisix": { "admin_api_version": "v2", @@ -212,6 +216,7 @@ kubernetes: election_id: my-election-id ingress_class: apisix ingress_version: networking/v1 + disable_status_updates: true apisix: admin_api_version: v2 default_cluster_base_url: {{.DEFAULT_CLUSTER_BASE_URL}} diff --git a/pkg/providers/apisix/apisix_cluster_config.go b/pkg/providers/apisix/apisix_cluster_config.go index b9ebcb6230..d5348fac00 100644 --- a/pkg/providers/apisix/apisix_cluster_config.go +++ b/pkg/providers/apisix/apisix_cluster_config.go @@ -434,6 +434,9 @@ func (c *apisixClusterConfigController) ResourceSync() { // recordStatus record resources status func (c *apisixClusterConfigController) recordStatus(at interface{}, reason string, err error, status metav1.ConditionStatus, generation int64) { + if c.Kubernetes.DisableStatusUpdates { + return + } // build condition message := utils.CommonSuccessMessage if err != nil { @@ -475,7 +478,7 @@ func (c *apisixClusterConfigController) recordStatus(at interface{}, reason stri conditions := make([]metav1.Condition, 0) v.Status.Conditions = conditions } - if utils.VerifyGeneration(&v.Status.Conditions, condition) { + if utils.VerifyConditions(&v.Status.Conditions, condition) { meta.SetStatusCondition(&v.Status.Conditions, condition) if _, errRecord := apisixClient.ApisixV2().ApisixClusterConfigs(). UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil { diff --git a/pkg/providers/apisix/apisix_consumer.go b/pkg/providers/apisix/apisix_consumer.go index 026453792e..0706f10bce 100644 --- a/pkg/providers/apisix/apisix_consumer.go +++ b/pkg/providers/apisix/apisix_consumer.go @@ -351,6 +351,9 @@ func (c *apisixConsumerController) ResourceSync() { // recordStatus record resources status func (c *apisixConsumerController) recordStatus(at interface{}, reason string, err error, status metav1.ConditionStatus, generation int64) { + if c.Kubernetes.DisableStatusUpdates { + return + } // build condition message := utils.CommonSuccessMessage if err != nil { diff --git a/pkg/providers/apisix/apisix_plugin_config.go b/pkg/providers/apisix/apisix_plugin_config.go index f6705710c1..1f60e59afa 100644 --- a/pkg/providers/apisix/apisix_plugin_config.go +++ b/pkg/providers/apisix/apisix_plugin_config.go @@ -392,6 +392,9 @@ func (c *apisixPluginConfigController) ResourceSync() { // recordStatus record resources status func (c *apisixPluginConfigController) recordStatus(at interface{}, reason string, err error, status metav1.ConditionStatus, generation int64) { + if c.Kubernetes.DisableStatusUpdates { + return + } // build condition message := utils.CommonSuccessMessage if err != nil { @@ -434,7 +437,7 @@ func (c *apisixPluginConfigController) recordStatus(at interface{}, reason strin conditions := make([]metav1.Condition, 0) v.Status.Conditions = conditions } - if utils.VerifyGeneration(&v.Status.Conditions, condition) { + if utils.VerifyConditions(&v.Status.Conditions, condition) { meta.SetStatusCondition(&v.Status.Conditions, condition) if _, errRecord := apisixClient.ApisixV2().ApisixPluginConfigs(v.Namespace). UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil { diff --git a/pkg/providers/apisix/apisix_route.go b/pkg/providers/apisix/apisix_route.go index 53113d8808..d051fb5a45 100644 --- a/pkg/providers/apisix/apisix_route.go +++ b/pkg/providers/apisix/apisix_route.go @@ -843,8 +843,20 @@ func (c *apisixRouteController) handleApisixUpstreamErr(ev *routeEvent, errOrigi c.workqueue.AddRateLimited(ev) } -// recordStatus record resources status +/* +recordStatus record resources status + +TODO: The resouceVersion of the sync phase and the recordStatus phase may be different. There is consistency +problem here, and incorrect status may be recorded.(It will only be triggered when it is updated multiple times in a short time) + + IsUpdateStatus(currentObject, latestObject) bool { + return currentObject.resourceVersion >= latestObject.resourceVersion && !Equal(currentObject.status, latestObject.status) + } +*/ func (c *apisixRouteController) recordStatus(at interface{}, reason string, err error, status metav1.ConditionStatus, generation int64) { + if c.Kubernetes.DisableStatusUpdates { + return + } // build condition message := utils.CommonSuccessMessage if err != nil { @@ -887,7 +899,7 @@ func (c *apisixRouteController) recordStatus(at interface{}, reason string, err conditions := make([]metav1.Condition, 0) v.Status.Conditions = conditions } - if utils.VerifyGeneration(&v.Status.Conditions, condition) { + if utils.VerifyConditions(&v.Status.Conditions, condition) && !meta.IsStatusConditionPresentAndEqual(v.Status.Conditions, condition.Type, condition.Status) { meta.SetStatusCondition(&v.Status.Conditions, condition) if _, errRecord := apisixClient.ApisixV2().ApisixRoutes(v.Namespace). UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil { diff --git a/pkg/providers/apisix/apisix_tls.go b/pkg/providers/apisix/apisix_tls.go index c5714cfc30..d10f5ca081 100644 --- a/pkg/providers/apisix/apisix_tls.go +++ b/pkg/providers/apisix/apisix_tls.go @@ -400,6 +400,9 @@ func (c *apisixTlsController) ResourceSync() { // recordStatus record resources status func (c *apisixTlsController) recordStatus(at interface{}, reason string, err error, status metav1.ConditionStatus, generation int64) { + if c.Kubernetes.DisableStatusUpdates { + return + } // build condition message := utils.CommonSuccessMessage if err != nil { @@ -442,7 +445,7 @@ func (c *apisixTlsController) recordStatus(at interface{}, reason string, err er conditions := make([]metav1.Condition, 0) v.Status.Conditions = conditions } - if utils.VerifyGeneration(&v.Status.Conditions, condition) { + if utils.VerifyConditions(&v.Status.Conditions, condition) { meta.SetStatusCondition(&v.Status.Conditions, condition) if _, errRecord := apisixClient.ApisixV2().ApisixTlses(v.Namespace). UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil { diff --git a/pkg/providers/apisix/apisix_upstream.go b/pkg/providers/apisix/apisix_upstream.go index 398032387d..afbfebe5a7 100644 --- a/pkg/providers/apisix/apisix_upstream.go +++ b/pkg/providers/apisix/apisix_upstream.go @@ -804,6 +804,9 @@ func (c *apisixUpstreamController) handleSvcErr(key string, errOrigin error) { // recordStatus record resources status func (c *apisixUpstreamController) recordStatus(at interface{}, reason string, err error, status metav1.ConditionStatus, generation int64) { + if c.Kubernetes.DisableStatusUpdates { + return + } // build condition message := utils.CommonSuccessMessage if err != nil { @@ -847,7 +850,7 @@ func (c *apisixUpstreamController) recordStatus(at interface{}, reason string, e conditions := make([]metav1.Condition, 0) v.Status.Conditions = conditions } - if utils.VerifyGeneration(&v.Status.Conditions, condition) { + if utils.VerifyConditions(&v.Status.Conditions, condition) { meta.SetStatusCondition(&v.Status.Conditions, condition) if _, errRecord := apisixClient.ApisixV2().ApisixUpstreams(v.Namespace). UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil { diff --git a/pkg/providers/controller.go b/pkg/providers/controller.go index 463609c0e9..a577063ddd 100644 --- a/pkg/providers/controller.go +++ b/pkg/providers/controller.go @@ -465,6 +465,7 @@ func (c *Controller) run(ctx context.Context) { KubeClient: c.kubeClient.Client, MetricsCollector: c.MetricsCollector, NamespaceProvider: c.namespaceProvider, + ListerInformer: common.ListerInformer, }) if err != nil { ctx.Done() diff --git a/pkg/providers/gateway/gateway.go b/pkg/providers/gateway/gateway.go index 6611f83939..e5c46dc156 100644 --- a/pkg/providers/gateway/gateway.go +++ b/pkg/providers/gateway/gateway.go @@ -246,7 +246,7 @@ func (c *gatewayController) recordStatus(v *gatewayv1beta1.Gateway, reason strin meta.SetStatusCondition(&v.Status.Conditions, gatewayCondition) } - lbips, err := utils.IngressLBStatusIPs(c.controller.Cfg.IngressPublishService, c.controller.Cfg.IngressStatusAddress, c.controller.KubeClient) + lbips, err := utils.IngressLBStatusIPs(c.controller.Cfg.IngressPublishService, c.controller.Cfg.IngressStatusAddress, c.controller.ListerInformer.SvcLister) if err != nil { log.Errorw("failed to get APISIX gateway external IPs", zap.Error(err), diff --git a/pkg/providers/gateway/provider.go b/pkg/providers/gateway/provider.go index 05663181e8..1c9773c24b 100644 --- a/pkg/providers/gateway/provider.go +++ b/pkg/providers/gateway/provider.go @@ -38,6 +38,7 @@ import ( "github.com/apache/apisix-ingress-controller/pkg/providers/gateway/types" "github.com/apache/apisix-ingress-controller/pkg/providers/k8s/namespace" "github.com/apache/apisix-ingress-controller/pkg/providers/translation" + providertypes "github.com/apache/apisix-ingress-controller/pkg/providers/types" "github.com/apache/apisix-ingress-controller/pkg/providers/utils" ) @@ -96,6 +97,7 @@ type ProviderOptions struct { KubeClient kubernetes.Interface MetricsCollector metrics.Collector NamespaceProvider namespace.WatchingNamespaceProvider + ListerInformer *providertypes.ListerInformer } func NewGatewayProvider(opts *ProviderOptions) (*Provider, error) { diff --git a/pkg/providers/ingress/ingress.go b/pkg/providers/ingress/ingress.go index e1de15ceb2..7c3d405bcb 100644 --- a/pkg/providers/ingress/ingress.go +++ b/pkg/providers/ingress/ingress.go @@ -465,6 +465,9 @@ func (c *ingressController) ResourceSync() { // recordStatus record resources status func (c *ingressController) recordStatus(at runtime.Object, reason string, err error, status metav1.ConditionStatus, generation int64) { + if c.Kubernetes.DisableStatusUpdates { + return + } client := c.KubeClient.Client at = at.DeepCopyObject() @@ -479,15 +482,16 @@ func (c *ingressController) recordStatus(at runtime.Object, reason string, err e ) } - - v.ObjectMeta.Generation = generation - v.Status.LoadBalancer.Ingress = utils.CoreV1ToNetworkV1LB(lbips) - if _, errRecord := client.NetworkingV1().Ingresses(v.Namespace).UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil { - log.Errorw("failed to record status change for IngressV1", - zap.Error(errRecord), - zap.String("name", v.Name), - zap.String("namespace", v.Namespace), - ) + ingressLB := utils.CoreV1ToNetworkV1LB(lbips) + if !utils.CompareNetworkingV1LBEqual(v.Status.LoadBalancer.Ingress, ingressLB) { + v.Status.LoadBalancer.Ingress = ingressLB + if _, errRecord := client.NetworkingV1().Ingresses(v.Namespace).UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil { + log.Errorw("failed to record status change for IngressV1", + zap.Error(errRecord), + zap.String("name", v.Name), + zap.String("namespace", v.Namespace), + ) + } } case *networkingv1beta1.Ingress: @@ -497,17 +501,18 @@ func (c *ingressController) recordStatus(at runtime.Object, reason string, err e log.Errorw("failed to get APISIX gateway external IPs", zap.Error(err), ) - } - v.ObjectMeta.Generation = generation - v.Status.LoadBalancer.Ingress = utils.CoreV1ToNetworkV1beta1LB(lbips) - if _, errRecord := client.NetworkingV1beta1().Ingresses(v.Namespace).UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil { - log.Errorw("failed to record status change for IngressV1", - zap.Error(errRecord), - zap.String("name", v.Name), - zap.String("namespace", v.Namespace), - ) + ingressLB := utils.CoreV1ToNetworkV1beta1LB(lbips) + if !utils.CompareNetworkingV1beta1LBEqual(v.Status.LoadBalancer.Ingress, ingressLB) { + v.Status.LoadBalancer.Ingress = ingressLB + if _, errRecord := client.NetworkingV1beta1().Ingresses(v.Namespace).UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil { + log.Errorw("failed to record status change for IngressV1beta1", + zap.Error(errRecord), + zap.String("name", v.Name), + zap.String("namespace", v.Namespace), + ) + } } case *extensionsv1beta1.Ingress: // set to status @@ -519,14 +524,16 @@ func (c *ingressController) recordStatus(at runtime.Object, reason string, err e } - v.ObjectMeta.Generation = generation - v.Status.LoadBalancer.Ingress = utils.CoreV1ToExtensionsV1beta1LB(lbips) - if _, errRecord := client.ExtensionsV1beta1().Ingresses(v.Namespace).UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil { - log.Errorw("failed to record status change for IngressV1", - zap.Error(errRecord), - zap.String("name", v.Name), - zap.String("namespace", v.Namespace), - ) + ingressLB := utils.CoreV1ToExtensionsV1beta1LB(lbips) + if !utils.CompareExtensionsV1beta1LBEqual(v.Status.LoadBalancer.Ingress, ingressLB) { + v.Status.LoadBalancer.Ingress = ingressLB + if _, errRecord := client.ExtensionsV1beta1().Ingresses(v.Namespace).UpdateStatus(context.TODO(), v, metav1.UpdateOptions{}); errRecord != nil { + log.Errorw("failed to record status change for IngressExtensionsv1beta1", + zap.Error(errRecord), + zap.String("name", v.Name), + zap.String("namespace", v.Namespace), + ) + } } default: // This should not be executed @@ -536,7 +543,7 @@ func (c *ingressController) recordStatus(at runtime.Object, reason string, err e // ingressLBStatusIPs organizes the available addresses func (c *ingressController) ingressLBStatusIPs() ([]corev1.LoadBalancerIngress, error) { - return utils.IngressLBStatusIPs(c.IngressPublishService, c.IngressStatusAddress, c.KubeClient.Client) + return utils.IngressLBStatusIPs(c.IngressPublishService, c.IngressStatusAddress, c.SvcLister) } func (c *ingressController) storeSecretReference(secretKey string, ingressKey string, evType types.EventType, ssl *v1.Ssl) { diff --git a/pkg/providers/utils/ingress_status.go b/pkg/providers/utils/ingress_status.go index 18d7c9b923..c106a19b0e 100644 --- a/pkg/providers/utils/ingress_status.go +++ b/pkg/providers/utils/ingress_status.go @@ -17,17 +17,17 @@ package utils import ( - "context" "fmt" "net" + "sort" + "strings" "time" corev1 "k8s.io/api/core/v1" extensionsv1beta1 "k8s.io/api/extensions/v1beta1" networkingv1 "k8s.io/api/networking/v1" networkingv1beta1 "k8s.io/api/networking/v1beta1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/kubernetes" + listerscorev1 "k8s.io/client-go/listers/core/v1" "k8s.io/client-go/tools/cache" gatewayv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" @@ -39,7 +39,7 @@ const ( ) // IngressPublishAddresses get addressed used to expose Ingress -func IngressPublishAddresses(ingressPublishService string, ingressStatusAddress []string, kubeClient kubernetes.Interface) ([]string, error) { +func IngressPublishAddresses(ingressPublishService string, ingressStatusAddress []string, svcLister listerscorev1.ServiceLister) ([]string, error) { addrs := []string{} // if ingressStatusAddress is specified, it will be used first @@ -54,7 +54,7 @@ func IngressPublishAddresses(ingressPublishService string, ingressStatusAddress return nil, err } - svc, err := kubeClient.CoreV1().Services(namespace).Get(context.TODO(), name, metav1.GetOptions{}) + svc, err := svcLister.Services(namespace).Get(name) if err != nil { return nil, err } @@ -83,13 +83,13 @@ func IngressPublishAddresses(ingressPublishService string, ingressStatusAddress } // IngressLBStatusIPs organizes the available addresses -func IngressLBStatusIPs(ingressPublishService string, ingressStatusAddress []string, kubeClient kubernetes.Interface) ([]corev1.LoadBalancerIngress, error) { +func IngressLBStatusIPs(ingressPublishService string, ingressStatusAddress []string, svcLister listerscorev1.ServiceLister) ([]corev1.LoadBalancerIngress, error) { lbips := []corev1.LoadBalancerIngress{} var ips []string for { var err error - ips, err = IngressPublishAddresses(ingressPublishService, ingressStatusAddress, kubeClient) + ips, err = IngressPublishAddresses(ingressPublishService, ingressStatusAddress, svcLister) if err != nil { if err.Error() == _gatewayLBNotReadyMessage { log.Warnf("%s. Provided service: %s", _gatewayLBNotReadyMessage, ingressPublishService) @@ -114,6 +114,96 @@ func IngressLBStatusIPs(ingressPublishService string, ingressStatusAddress []str return lbips, nil } +func lessNetworkingV1LB(addrs []networkingv1.IngressLoadBalancerIngress) func(int, int) bool { + return func(a, b int) bool { + switch strings.Compare(addrs[a].Hostname, addrs[b].Hostname) { + case -1: + return true + case 1: + return false + } + return addrs[a].IP < addrs[b].IP + } +} + +func lessNetworkingV1beta1LB(addrs []networkingv1beta1.IngressLoadBalancerIngress) func(int, int) bool { + return func(a, b int) bool { + switch strings.Compare(addrs[a].Hostname, addrs[b].Hostname) { + case -1: + return true + case 1: + return false + } + return addrs[a].IP < addrs[b].IP + } +} + +func lessExtensionsV1beta1LB(addrs []extensionsv1beta1.IngressLoadBalancerIngress) func(int, int) bool { + return func(a, b int) bool { + switch strings.Compare(addrs[a].Hostname, addrs[b].Hostname) { + case -1: + return true + case 1: + return false + } + return addrs[a].IP < addrs[b].IP + } +} + +func CompareNetworkingV1LBEqual(lb1 []networkingv1.IngressLoadBalancerIngress, lb2 []networkingv1.IngressLoadBalancerIngress) bool { + if len(lb1) != len(lb2) { + return false + } + sort.SliceStable(lb1, lessNetworkingV1LB(lb1)) + sort.SliceStable(lb2, lessNetworkingV1LB(lb2)) + size := len(lb1) + for i := 0; i < size; i++ { + if lb1[i].IP != lb2[i].IP { + return false + } + if lb1[i].Hostname != lb2[i].Hostname { + return false + } + } + return true +} + +func CompareNetworkingV1beta1LBEqual(lb1 []networkingv1beta1.IngressLoadBalancerIngress, lb2 []networkingv1beta1.IngressLoadBalancerIngress) bool { + if len(lb1) != len(lb2) { + return false + } + sort.SliceStable(lb1, lessNetworkingV1beta1LB(lb1)) + sort.SliceStable(lb2, lessNetworkingV1beta1LB(lb2)) + size := len(lb1) + for i := 0; i < size; i++ { + if lb1[i].IP != lb2[i].IP { + return false + } + if lb1[i].Hostname != lb2[i].Hostname { + return false + } + } + return true +} + +func CompareExtensionsV1beta1LBEqual(lb1 []extensionsv1beta1.IngressLoadBalancerIngress, lb2 []extensionsv1beta1.IngressLoadBalancerIngress) bool { + if len(lb1) != len(lb2) { + return false + } + sort.SliceStable(lb1, lessExtensionsV1beta1LB(lb1)) + sort.SliceStable(lb2, lessExtensionsV1beta1LB(lb2)) + size := len(lb1) + for i := 0; i < size; i++ { + if lb1[i].IP != lb2[i].IP { + return false + } + if lb1[i].Hostname != lb2[i].Hostname { + return false + } + } + return true +} + // CoreV1ToNetworkV1LB convert []corev1.LoadBalancerIngress to []networkingv1.IngressLoadBalancerIngress func CoreV1ToNetworkV1LB(lbips []corev1.LoadBalancerIngress) []networkingv1.IngressLoadBalancerIngress { t := make([]networkingv1.IngressLoadBalancerIngress, 0, len(lbips)) diff --git a/pkg/providers/utils/ingress_status_test.go b/pkg/providers/utils/ingress_status_test.go new file mode 100644 index 0000000000..cb2b1bab01 --- /dev/null +++ b/pkg/providers/utils/ingress_status_test.go @@ -0,0 +1,257 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package utils + +import ( + "testing" + + "github.com/stretchr/testify/assert" + extensionsv1beta1 "k8s.io/api/extensions/v1beta1" + networkingv1 "k8s.io/api/networking/v1" + networkingv1beta1 "k8s.io/api/networking/v1beta1" +) + +func TestCompareNetworkingV1LBEqual(t *testing.T) { + lb1 := []networkingv1.IngressLoadBalancerIngress{ + { + IP: "0.0.0.0", + }, + } + lb2 := []networkingv1.IngressLoadBalancerIngress{ + { + IP: "0.0.0.0", + }, + } + assert.Equal(t, true, CompareNetworkingV1LBEqual(lb1, lb2)) + + lb1 = []networkingv1.IngressLoadBalancerIngress{ + { + IP: "0.0.0.0", + }, + } + lb2 = []networkingv1.IngressLoadBalancerIngress{ + { + IP: "0.0.0.0", + }, + { + Hostname: "test.com", + }, + } + assert.Equal(t, false, CompareNetworkingV1LBEqual(lb1, lb2)) + + lb1 = []networkingv1.IngressLoadBalancerIngress{ + { + IP: "127.0.0.1", + }, + { + IP: "0.0.0.0", + }, + } + lb2 = []networkingv1.IngressLoadBalancerIngress{ + { + IP: "0.0.0.0", + }, + { + IP: "127.0.0.1", + }, + } + assert.Equal(t, true, CompareNetworkingV1LBEqual(lb1, lb2)) + + lb1 = []networkingv1.IngressLoadBalancerIngress{ + { + IP: "127.0.0.1", + }, + { + IP: "1.1.1.1", + }, + } + lb2 = []networkingv1.IngressLoadBalancerIngress{ + { + IP: "127.0.0.1", + }, + { + IP: "0.0.0.0", + }, + } + assert.Equal(t, false, CompareNetworkingV1LBEqual(lb1, lb2)) + + lb1 = []networkingv1.IngressLoadBalancerIngress{ + { + Hostname: "test.com", + }, + { + IP: "127.0.0.1", + }, + { + IP: "0.0.0.0", + }, + } + lb2 = []networkingv1.IngressLoadBalancerIngress{ + { + IP: "0.0.0.0", + }, + { + IP: "127.0.0.1", + }, + { + Hostname: "test.com", + }, + } + assert.Equal(t, true, CompareNetworkingV1LBEqual(lb1, lb2)) +} + +func TestCompareNetworkingV1beta1LBEqual(t *testing.T) { + lb1 := []networkingv1beta1.IngressLoadBalancerIngress{ + { + IP: "0.0.0.0", + }, + } + lb2 := []networkingv1beta1.IngressLoadBalancerIngress{ + { + IP: "0.0.0.0", + }, + } + assert.Equal(t, true, CompareNetworkingV1beta1LBEqual(lb1, lb2)) + + lb1 = []networkingv1beta1.IngressLoadBalancerIngress{ + { + IP: "0.0.0.0", + }, + } + lb2 = []networkingv1beta1.IngressLoadBalancerIngress{ + { + IP: "0.0.0.0", + }, + { + Hostname: "test.com", + }, + } + assert.Equal(t, false, CompareNetworkingV1beta1LBEqual(lb1, lb2)) + + lb1 = []networkingv1beta1.IngressLoadBalancerIngress{ + { + IP: "127.0.0.1", + }, + { + IP: "1.1.1.1", + }, + } + lb2 = []networkingv1beta1.IngressLoadBalancerIngress{ + { + IP: "127.0.0.1", + }, + { + IP: "0.0.0.0", + }, + } + assert.Equal(t, false, CompareNetworkingV1beta1LBEqual(lb1, lb2)) + + lb1 = []networkingv1beta1.IngressLoadBalancerIngress{ + { + Hostname: "test.com", + }, + { + IP: "127.0.0.1", + }, + { + IP: "0.0.0.0", + }, + } + lb2 = []networkingv1beta1.IngressLoadBalancerIngress{ + { + IP: "0.0.0.0", + }, + { + IP: "127.0.0.1", + }, + { + Hostname: "test.com", + }, + } + assert.Equal(t, true, CompareNetworkingV1beta1LBEqual(lb1, lb2)) +} + +func TestCompareExtensionsV1beta1LBEqual(t *testing.T) { + lb1 := []extensionsv1beta1.IngressLoadBalancerIngress{ + { + IP: "0.0.0.0", + }, + } + lb2 := []extensionsv1beta1.IngressLoadBalancerIngress{ + { + IP: "0.0.0.0", + }, + } + assert.Equal(t, true, CompareExtensionsV1beta1LBEqual(lb1, lb2)) + + lb1 = []extensionsv1beta1.IngressLoadBalancerIngress{ + { + IP: "0.0.0.0", + }, + } + lb2 = []extensionsv1beta1.IngressLoadBalancerIngress{ + { + IP: "0.0.0.0", + }, + { + Hostname: "test.com", + }, + } + assert.Equal(t, false, CompareExtensionsV1beta1LBEqual(lb1, lb2)) + + lb1 = []extensionsv1beta1.IngressLoadBalancerIngress{ + { + IP: "127.0.0.1", + }, + { + IP: "1.1.1.1", + }, + } + lb2 = []extensionsv1beta1.IngressLoadBalancerIngress{ + { + IP: "127.0.0.1", + }, + { + IP: "0.0.0.0", + }, + } + assert.Equal(t, false, CompareExtensionsV1beta1LBEqual(lb1, lb2)) + + lb1 = []extensionsv1beta1.IngressLoadBalancerIngress{ + { + Hostname: "test.com", + }, + { + IP: "127.0.0.1", + }, + { + IP: "0.0.0.0", + }, + } + lb2 = []extensionsv1beta1.IngressLoadBalancerIngress{ + { + IP: "0.0.0.0", + }, + { + IP: "127.0.0.1", + }, + { + Hostname: "test.com", + }, + } + assert.Equal(t, true, CompareExtensionsV1beta1LBEqual(lb1, lb2)) +} diff --git a/pkg/providers/utils/status.go b/pkg/providers/utils/status.go index 3f624aa1c4..12e73f4325 100644 --- a/pkg/providers/utils/status.go +++ b/pkg/providers/utils/status.go @@ -64,3 +64,19 @@ func VerifyGeneration(conditions *[]metav1.Condition, newCondition metav1.Condit } return true } + +// VerifyConditions verify conditions to decide whether to update status +func VerifyConditions(conditions *[]metav1.Condition, newCondition metav1.Condition) bool { + existingCondition := meta.FindStatusCondition(*conditions, newCondition.Type) + if existingCondition == nil { + return true + } + + if existingCondition.ObservedGeneration > newCondition.ObservedGeneration { + return false + } + if *existingCondition == newCondition { + return false + } + return true +} diff --git a/pkg/providers/utils/status_test.go b/pkg/providers/utils/status_test.go new file mode 100644 index 0000000000..7f90e98067 --- /dev/null +++ b/pkg/providers/utils/status_test.go @@ -0,0 +1,102 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package utils + +import ( + "testing" + + "github.com/stretchr/testify/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestVerifyConditions(t *testing.T) { + // Different status + conditions := []metav1.Condition{ + { + Type: ConditionType, + Status: metav1.ConditionTrue, + ObservedGeneration: 1, + }, + } + newCondition := metav1.Condition{ + Type: ConditionType, + Status: metav1.ConditionFalse, + ObservedGeneration: 1, + } + assert.Equal(t, true, VerifyConditions(&conditions, newCondition)) + + // same condition + conditions = []metav1.Condition{ + { + Type: ConditionType, + Status: metav1.ConditionTrue, + ObservedGeneration: 1, + }, + } + newCondition = metav1.Condition{ + Type: ConditionType, + Status: metav1.ConditionTrue, + ObservedGeneration: 1, + } + assert.Equal(t, false, VerifyConditions(&conditions, newCondition)) + + // Different ObservedGeneration + conditions = []metav1.Condition{ + { + Type: ConditionType, + Status: metav1.ConditionTrue, + ObservedGeneration: 1, + }, + } + newCondition = metav1.Condition{ + Type: ConditionType, + Status: metav1.ConditionTrue, + ObservedGeneration: 2, + } + assert.Equal(t, true, VerifyConditions(&conditions, newCondition)) + + conditions = []metav1.Condition{ + { + Type: ConditionType, + Status: metav1.ConditionTrue, + ObservedGeneration: 2, + }, + } + newCondition = metav1.Condition{ + Type: ConditionType, + Status: metav1.ConditionTrue, + ObservedGeneration: 1, + } + assert.Equal(t, false, VerifyConditions(&conditions, newCondition)) + + // Different message + conditions = []metav1.Condition{ + { + Type: ConditionType, + Status: metav1.ConditionFalse, + Message: "port does not exist", + ObservedGeneration: 1, + }, + } + newCondition = metav1.Condition{ + Type: ConditionType, + Status: metav1.ConditionFalse, + Message: "service does not exist", + ObservedGeneration: 1, + } + assert.Equal(t, true, VerifyConditions(&conditions, newCondition)) +} diff --git a/test/e2e/scaffold/ingress.go b/test/e2e/scaffold/ingress.go index 87e639d93e..21b50e7c4f 100644 --- a/test/e2e/scaffold/ingress.go +++ b/test/e2e/scaffold/ingress.go @@ -398,6 +398,7 @@ spec: - "%s" - --enable-gateway-api - "true" + %s %s volumes: - name: webhook-certs @@ -433,20 +434,25 @@ func (s *Scaffold) newIngressAPISIXController() error { assert.Nil(s.t, err, "deleting ClusterRole") }) - var ingressAPISIXDeployment string + var ( + ingressAPISIXDeployment string + disableStatusStr string + webhookVolumeMounts string + ) label := `""` if labels := s.NamespaceSelectorLabelStrings(); labels != nil && !s.opts.DisableNamespaceSelector { label = labels[0] } - + if s.opts.DisableStatus { + disableStatusStr = "- --disable-status-updates" + } if s.opts.EnableWebhooks { - ingressAPISIXDeployment = fmt.Sprintf(s.FormatRegistry(_ingressAPISIXDeploymentTemplate), s.opts.IngressAPISIXReplicas, s.namespace, s.opts.APISIXAdminAPIVersion, s.opts.ApisixResourceSyncInterval, - label, s.opts.ApisixResourceVersion, s.opts.APISIXPublishAddress, _volumeMounts, _webhookCertSecret) - } else { - ingressAPISIXDeployment = fmt.Sprintf(s.FormatRegistry(_ingressAPISIXDeploymentTemplate), s.opts.IngressAPISIXReplicas, s.namespace, s.opts.APISIXAdminAPIVersion, s.opts.ApisixResourceSyncInterval, - label, s.opts.ApisixResourceVersion, s.opts.APISIXPublishAddress, "", _webhookCertSecret) + webhookVolumeMounts = _volumeMounts } + ingressAPISIXDeployment = fmt.Sprintf(s.FormatRegistry(_ingressAPISIXDeploymentTemplate), s.opts.IngressAPISIXReplicas, s.namespace, s.opts.APISIXAdminAPIVersion, s.opts.ApisixResourceSyncInterval, + label, s.opts.ApisixResourceVersion, s.opts.APISIXPublishAddress, disableStatusStr, webhookVolumeMounts, _webhookCertSecret) + err = s.CreateResourceFromString(ingressAPISIXDeployment) assert.Nil(s.t, err, "create deployment") @@ -547,16 +553,27 @@ func (s *Scaffold) GetIngressPodDetails() ([]corev1.Pod, error) { // ScaleIngressController scales the number of Ingress Controller pods to desired. func (s *Scaffold) ScaleIngressController(desired int) error { - var ingressDeployment string - var label string + var ( + ingressDeployment string + label string + disableStatusStr string + webhookVolumeMounts string + ) + if labels := s.NamespaceSelectorLabelStrings(); labels != nil { label = labels[0] } + if s.opts.DisableStatus { + disableStatusStr = "- --disable-status-updates" + } if s.opts.EnableWebhooks { - ingressDeployment = fmt.Sprintf(s.FormatRegistry(_ingressAPISIXDeploymentTemplate), desired, s.namespace, s.opts.APISIXAdminAPIVersion, s.opts.ApisixResourceSyncInterval, label, s.opts.ApisixResourceVersion, s.opts.APISIXPublishAddress, _volumeMounts, _webhookCertSecret) - } else { - ingressDeployment = fmt.Sprintf(s.FormatRegistry(_ingressAPISIXDeploymentTemplate), desired, s.namespace, s.opts.APISIXAdminAPIVersion, s.opts.ApisixResourceSyncInterval, label, s.opts.ApisixResourceVersion, s.opts.APISIXPublishAddress, "", _webhookCertSecret) + webhookVolumeMounts = _volumeMounts } + + ingressDeployment = fmt.Sprintf(s.FormatRegistry(_ingressAPISIXDeploymentTemplate), desired, s.namespace, + s.opts.APISIXAdminAPIVersion, s.opts.ApisixResourceSyncInterval, label, s.opts.ApisixResourceVersion, s.opts.APISIXPublishAddress, + disableStatusStr, webhookVolumeMounts, _webhookCertSecret) + if err := s.CreateResourceFromString(ingressDeployment); err != nil { return err } diff --git a/test/e2e/scaffold/scaffold.go b/test/e2e/scaffold/scaffold.go index ce9869b2be..cf15b8a208 100644 --- a/test/e2e/scaffold/scaffold.go +++ b/test/e2e/scaffold/scaffold.go @@ -59,6 +59,7 @@ type Options struct { APISIXPublishAddress string ApisixResourceSyncInterval string ApisixResourceVersion string + DisableStatus bool NamespaceSelectorLabel map[string]string DisableNamespaceSelector bool diff --git a/test/e2e/suite-ingress/suite-ingress-features/status.go b/test/e2e/suite-ingress/suite-ingress-features/status.go index 7ddd1a0db7..5a2f1d71a9 100644 --- a/test/e2e/suite-ingress/suite-ingress-features/status.go +++ b/test/e2e/suite-ingress/suite-ingress-features/status.go @@ -144,3 +144,77 @@ spec: assert.True(ginkgo.GinkgoT(), hasIP, "LB Status is recorded") }) }) + +var _ = ginkgo.Describe("suite-ingress-features: disable status", func() { + opts := &scaffold.Options{ + Name: "default", + IngressAPISIXReplicas: 1, + APISIXPublishAddress: "10.6.6.6", + DisableStatus: true, + } + s := scaffold.NewScaffold(opts) + ginkgo.It("check the ApisixRoute status is recorded", func() { + backendSvc, backendSvcPort := s.DefaultHTTPBackend() + apisixRoute := fmt.Sprintf(` +apiVersion: apisix.apache.org/v2 +kind: ApisixRoute +metadata: + name: httpbin-route +spec: + http: + - name: rule1 + match: + hosts: + - httpbin.com + paths: + - /ip + backends: + - serviceName: %s + servicePort: %d +`, backendSvc, backendSvcPort[0]) + assert.Nil(ginkgo.GinkgoT(), s.CreateVersionedApisixResource(apisixRoute)) + + err := s.EnsureNumApisixRoutesCreated(1) + assert.Nil(ginkgo.GinkgoT(), err, "Checking number of routes") + err = s.EnsureNumApisixUpstreamsCreated(1) + assert.Nil(ginkgo.GinkgoT(), err, "Checking number of upstreams") + // status should be recorded as successful + output, err := s.GetOutputFromString("ar", "httpbin-route", "-o", "jsonpath='{ .status }'") + assert.Nil(ginkgo.GinkgoT(), err) + assert.Equal(ginkgo.GinkgoT(), "''", output) + }) + + ginkgo.It("check the ingress lb status is updated", func() { + backendSvc, backendPort := s.DefaultHTTPBackend() + ing := fmt.Sprintf(` +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + annotations: + kubernetes.io/ingress.class: apisix + name: ingress-v1-lb +spec: + rules: + - host: httpbin.org + http: + paths: + - path: /ip + pathType: Exact + backend: + service: + name: %s + port: + number: %d +`, backendSvc, backendPort[0]) + err := s.CreateResourceFromString(ing) + assert.Nil(ginkgo.GinkgoT(), err, "creating ingress") + time.Sleep(5 * time.Second) + + _ = s.NewAPISIXClient().GET("/ip").WithHeader("Host", "httpbin.org").Expect().Status(http.StatusOK) + + output, err := s.GetOutputFromString("ingress", "ingress-v1-lb", "-o", "jsonpath='{ .status.loadBalancer }'") + assert.Nil(ginkgo.GinkgoT(), err, "Get output of ingress status") + + assert.Equal(ginkgo.GinkgoT(), "'{}'", output) + }) +})