From f6bd4f658c9a5819a63d65aa9471b0ff60496dc5 Mon Sep 17 00:00:00 2001 From: chiragkyal Date: Wed, 25 Sep 2024 13:46:25 +0530 Subject: [PATCH 1/3] Add support for AWS user tags from PlatformStatus - Added watch on Infrastructure object to detect changes in PlatformStatus.AWS.ResourceTags. - Merged tags from AWSLoadBalancerController.Spec.AdditionalResourceTags and PlatformStatus.AWS.ResourceTags, prioritizing operator spec tags. Signed-off-by: chiragkyal --- Makefile | 14 +- .../awsloadbalancercontroller/controller.go | 36 +- .../awsloadbalancercontroller/deployment.go | 72 +- .../deployment_test.go | 152 +++- pkg/controllers/suite_test.go | 5 + pkg/controllers/watch_test.go | 33 + .../test/crd/infrastructure-Default.yaml | 855 ++++++++++++++++++ 7 files changed, 1130 insertions(+), 37 deletions(-) create mode 100644 pkg/utils/test/crd/infrastructure-Default.yaml diff --git a/Makefile b/Makefile index 75cdb967..54be021f 100644 --- a/Makefile +++ b/Makefile @@ -120,6 +120,9 @@ help: ## Display this help. ##@ Development +.PHONY: update +update: update-vendored-crds manifests generate + .PHONY: manifests manifests: ## Generate WebhookConfiguration, ClusterRole and CustomResourceDefinition objects. $(CONTROLLER_GEN) rbac:roleName=manager-role crd webhook paths="./..." output:crd:artifacts:config=config/crd/bases @@ -130,6 +133,11 @@ manifests: ## Generate WebhookConfiguration, ClusterRole and CustomResourceDefin generate: iamctl-gen iam-gen## Generate code containing DeepCopy, DeepCopyInto, DeepCopyObject method implementations and iamctl policies. $(CONTROLLER_GEN) object:headerFile="hack/boilerplate.go.txt" paths="./..." +.PHONY: update-vendored-crds +update-vendored-crds: + ## Copy infrastructure CRD from openshift/api + cp vendor/github.com/openshift/api/config/v1/0000_10_config-operator_01_infrastructure-Default.crd.yaml ./pkg/utils/test/crd/infrastructure-Default.yaml + .PHONY: fmt fmt: ## Run go fmt against code. go fmt -mod=vendor ./... @@ -301,8 +309,12 @@ catalog-build: catalog catalog-push: $(MAKE) image-push IMG=$(CATALOG_IMG) +.PHONY: verify-vendored-crds +verify-vendored-crds: + diff vendor/github.com/openshift/api/config/v1/0000_10_config-operator_01_infrastructure-Default.crd.yaml ./pkg/utils/test/crd/infrastructure-Default.yaml + .PHONY: verify -verify: +verify: verify-vendored-crds hack/verify-deps.sh hack/verify-generated.sh hack/verify-gofmt.sh diff --git a/pkg/controllers/awsloadbalancercontroller/controller.go b/pkg/controllers/awsloadbalancercontroller/controller.go index e408b5a5..6bc710b9 100644 --- a/pkg/controllers/awsloadbalancercontroller/controller.go +++ b/pkg/controllers/awsloadbalancercontroller/controller.go @@ -31,6 +31,7 @@ import ( cco "github.com/openshift/cloud-credential-operator/pkg/apis/cloudcredential/v1" + configv1 "github.com/openshift/api/config/v1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" @@ -46,6 +47,8 @@ import ( const ( // the name of the AWSLoadBalancerController resource which will be reconciled controllerName = "cluster" + // clusterInfrastructureName is the name of the 'cluster' infrastructure object. + clusterInfrastructureName = "cluster" // the port on which controller metrics are served controllerMetricsPort = 8080 // the port on which the controller webhook is served @@ -120,6 +123,15 @@ func (r *AWSLoadBalancerControllerReconciler) Reconcile(ctx context.Context, req } } + infraConfig := &configv1.Infrastructure{} + if err := r.Client.Get(ctx, types.NamespacedName{Name: clusterInfrastructureName}, infraConfig); err != nil { + return ctrl.Result{}, fmt.Errorf("failed to get infrastructure %q: %w", clusterInfrastructureName, err) + } + platformStatus := infraConfig.Status.PlatformStatus + if platformStatus == nil { + return ctrl.Result{}, fmt.Errorf("failed to determine infrastructure platform status: status.platformStatus is nil") + } + if err := r.ensureIngressClass(ctx, lbController); err != nil { return ctrl.Result{}, fmt.Errorf("failed to ensure default IngressClass for AWSLoadBalancerController %q: %v", req.Name, err) } @@ -187,7 +199,7 @@ func (r *AWSLoadBalancerControllerReconciler) Reconcile(ctx context.Context, req return ctrl.Result{}, fmt.Errorf("failed to ensure ClusterRole and Binding for AWSLoadBalancerController %q: %w", req.Name, err) } - deployment, err := r.ensureDeployment(ctx, sa, credSecretNsName.Name, servingSecretName, lbController, trustCAConfigMap) + deployment, err := r.ensureDeployment(ctx, sa, credSecretNsName.Name, servingSecretName, lbController, platformStatus, trustCAConfigMap) if err != nil { return ctrl.Result{}, fmt.Errorf("failed to ensure Deployment for AWSLoadbalancerController %q: %w", req.Name, err) } @@ -247,17 +259,17 @@ func (r *AWSLoadBalancerControllerReconciler) BuildManagedController(mgr ctrl.Ma Owns(&arv1.ValidatingWebhookConfiguration{}). Owns(&arv1.MutatingWebhookConfiguration{}) - if r.TrustedCAConfigMapName != "" { - clusterALBCInstance := func(ctx context.Context, o client.Object) []reconcile.Request { - return []reconcile.Request{ - { - NamespacedName: types.NamespacedName{ - Name: controllerName, - }, + clusterALBCInstance := func(ctx context.Context, o client.Object) []reconcile.Request { + return []reconcile.Request{ + { + NamespacedName: types.NamespacedName{ + Name: controllerName, }, - } + }, } + } + if r.TrustedCAConfigMapName != "" { // Requeue the only (cluster) instance of AWSLoadBalancerController // so that the main reconciliation loop can detect the changes in the trusted CA configmap's contents // and redeploy the controller if needed. @@ -270,6 +282,12 @@ func (r *AWSLoadBalancerControllerReconciler) BuildManagedController(mgr ctrl.Ma predicate.NewPredicateFuncs(inNamespace(r.Namespace))), predicate.NewPredicateFuncs(hasName(r.TrustedCAConfigMapName)))) } + // Watch Infrastructure object to detect changes in AWS user tags + bldr = bldr.Watches(&configv1.Infrastructure{}, + handler.EnqueueRequestsFromMapFunc(clusterALBCInstance), + builder.WithPredicates( + predicate.NewPredicateFuncs(hasName(clusterInfrastructureName)))) + return bldr } diff --git a/pkg/controllers/awsloadbalancercontroller/deployment.go b/pkg/controllers/awsloadbalancercontroller/deployment.go index bd2a7840..c12db7d2 100644 --- a/pkg/controllers/awsloadbalancercontroller/deployment.go +++ b/pkg/controllers/awsloadbalancercontroller/deployment.go @@ -21,6 +21,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/log" + configv1 "github.com/openshift/api/config/v1" + albo "github.com/openshift/aws-load-balancer-operator/api/v1" ) @@ -69,7 +71,7 @@ const ( allCapabilities = "ALL" ) -func (r *AWSLoadBalancerControllerReconciler) ensureDeployment(ctx context.Context, sa *corev1.ServiceAccount, crSecretName, servingSecretName string, controller *albo.AWSLoadBalancerController, trustCAConfigMap *corev1.ConfigMap) (*appsv1.Deployment, error) { +func (r *AWSLoadBalancerControllerReconciler) ensureDeployment(ctx context.Context, sa *corev1.ServiceAccount, crSecretName, servingSecretName string, controller *albo.AWSLoadBalancerController, platformStatus *configv1.PlatformStatus, trustCAConfigMap *corev1.ConfigMap) (*appsv1.Deployment, error) { deploymentName := fmt.Sprintf("%s-%s", controllerResourcePrefix, controller.Name) reqLogger := log.FromContext(ctx).WithValues("deployment", deploymentName) @@ -90,7 +92,11 @@ func (r *AWSLoadBalancerControllerReconciler) ensureDeployment(ctx context.Conte trustCAConfigMapHash = configMapHash } - desired := r.desiredDeployment(deploymentName, crSecretName, servingSecretName, controller, sa, trustCAConfigMapName, trustCAConfigMapHash) + desired, err := r.desiredDeployment(deploymentName, crSecretName, servingSecretName, controller, platformStatus, sa, trustCAConfigMapName, trustCAConfigMapHash) + if err != nil { + return nil, fmt.Errorf("failed to get desired deployment %s: %w", deploymentName, err) + } + err = controllerutil.SetControllerReference(controller, desired, r.Scheme) if err != nil { return nil, fmt.Errorf("failed to set owner reference on deployment %s: %w", deploymentName, err) @@ -120,7 +126,11 @@ func (r *AWSLoadBalancerControllerReconciler) ensureDeployment(ctx context.Conte return current, nil } -func (r *AWSLoadBalancerControllerReconciler) desiredDeployment(name, credentialsRequestSecretName, servingSecret string, controller *albo.AWSLoadBalancerController, sa *corev1.ServiceAccount, trustedCAConfigMapName, trustedCAConfigMapHash string) *appsv1.Deployment { +func (r *AWSLoadBalancerControllerReconciler) desiredDeployment(name, credentialsRequestSecretName, servingSecret string, controller *albo.AWSLoadBalancerController, platformStatus *configv1.PlatformStatus, sa *corev1.ServiceAccount, trustedCAConfigMapName, trustedCAConfigMapHash string) (*appsv1.Deployment, error) { + containerArgs, err := desiredContainerArgs(controller, platformStatus, r.ClusterName, r.VPCID) + if err != nil { + return nil, fmt.Errorf("failed to get container args: %w", err) + } d := &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ Name: name, @@ -145,7 +155,7 @@ func (r *AWSLoadBalancerControllerReconciler) desiredDeployment(name, credential { Name: awsLoadBalancerControllerContainerName, Image: r.Image, - Args: desiredContainerArgs(controller, r.ClusterName, r.VPCID), + Args: containerArgs, Env: append([]corev1.EnvVar{ { Name: awsRegionEnvVarName, @@ -257,21 +267,23 @@ func (r *AWSLoadBalancerControllerReconciler) desiredDeployment(name, credential }) } } - return d + return d, nil } -func desiredContainerArgs(controller *albo.AWSLoadBalancerController, clusterName, vpcID string) []string { +func desiredContainerArgs(controller *albo.AWSLoadBalancerController, platformStatus *configv1.PlatformStatus, clusterName, vpcID string) ([]string, error) { var args []string args = append(args, fmt.Sprintf("--webhook-cert-dir=%s", webhookTLSDir)) args = append(args, fmt.Sprintf("--aws-vpc-id=%s", vpcID)) args = append(args, fmt.Sprintf("--cluster-name=%s", clusterName)) - // if additional keys are present then sort them and append it to the arguments - if controller.Spec.AdditionalResourceTags != nil { - var tags []string - for _, t := range controller.Spec.AdditionalResourceTags { - tags = append(tags, fmt.Sprintf("%s=%s", t.Key, t.Value)) - } + tags := mergeTags(controller, platformStatus) + // `--default-tags` arg allows a maximum of 24 user tags, but the combination of + // controller.Spec.AdditionalResourceTags and platformStatus.AWS.ResourceTags can result in more. + // Ensure a maximum of 24 merged tags are added. + if len(tags) > 24 { + return nil, fmt.Errorf("exceeded maximum of 24 allowed tags, got %d", len(tags)) + } + if len(tags) > 0 { sort.Strings(tags) args = append(args, fmt.Sprintf(`--default-tags=%s`, strings.Join(tags, ","))) } @@ -302,7 +314,7 @@ func desiredContainerArgs(controller *albo.AWSLoadBalancerController, clusterNam args = append(args, fmt.Sprintf("--ingress-class=%s", controller.Spec.IngressClass)) args = append(args, "--feature-gates=EnableIPTargetType=false") sort.Strings(args) - return args + return args, nil } func (r *AWSLoadBalancerControllerReconciler) currentDeployment(ctx context.Context, name string, namespace string) (bool, *appsv1.Deployment, error) { @@ -542,3 +554,37 @@ func buildMapHash(data map[string]string) (string, error) { } return hex.EncodeToString(hash.Sum(nil)), nil } + +// mergeTags merges tags from an AWSLoadBalancerController and PlatformStatus into a slice of strings. +// Tags from `controller.Spec.AdditionalResourceTags` take precedence over those in `platformStatus.AWS.ResourceTags`. +// If a tag key already exists, the value from AdditionalResourceTags will overwrite the one from ResourceTags. +func mergeTags(controller *albo.AWSLoadBalancerController, platformStatus *configv1.PlatformStatus) []string { + // tagMap holds tags with unique keys + tagMap := make(map[string]string) + + // Add tags from controller.Spec.AdditionalResourceTags since operator tags has higher precedence + if controller.Spec.AdditionalResourceTags != nil { + for _, t := range controller.Spec.AdditionalResourceTags { + tagMap[t.Key] = t.Value + } + } + + // Add tags from platformStatus.AWS.ResourceTags to the map, only if the key doesn't exist + if platformStatus.AWS != nil && len(platformStatus.AWS.ResourceTags) > 0 { + for _, t := range platformStatus.AWS.ResourceTags { + if len(t.Key) > 0 { + if _, exists := tagMap[t.Key]; !exists { + tagMap[t.Key] = t.Value + } + } + } + } + + // Convert map back to a slice + var tags []string + for key, value := range tagMap { + tags = append(tags, fmt.Sprintf("%s=%s", key, value)) + } + + return tags +} diff --git a/pkg/controllers/awsloadbalancercontroller/deployment_test.go b/pkg/controllers/awsloadbalancercontroller/deployment_test.go index 8d99b072..e65d2b8c 100644 --- a/pkg/controllers/awsloadbalancercontroller/deployment_test.go +++ b/pkg/controllers/awsloadbalancercontroller/deployment_test.go @@ -18,6 +18,8 @@ import ( "github.com/google/go-cmp/cmp" "sigs.k8s.io/controller-runtime/pkg/client/fake" + configv1 "github.com/openshift/api/config/v1" + albo "github.com/openshift/aws-load-balancer-operator/api/v1" "github.com/openshift/aws-load-balancer-operator/pkg/utils/test" ) @@ -28,9 +30,11 @@ const ( func TestDesiredArgs(t *testing.T) { for _, tc := range []struct { - name string - controller *albo.AWSLoadBalancerController - expectedArgs sets.Set[string] + name string + controller *albo.AWSLoadBalancerController + platformStatus *configv1.PlatformStatus + expectedArgs sets.Set[string] + expectedError bool }{ { name: "non-default ingress class", @@ -110,7 +114,7 @@ func TestDesiredArgs(t *testing.T) { ), }, { - name: "resource tags specified", + name: "resource tags specified in the operator spec", controller: &albo.AWSLoadBalancerController{ Spec: albo.AWSLoadBalancerControllerSpec{ AdditionalResourceTags: []albo.AWSResourceTag{ @@ -128,6 +132,99 @@ func TestDesiredArgs(t *testing.T) { "--default-tags=test-key1=test-value1,test-key2=test-value2,test-key3=test-value3", ), }, + { + name: "resource tags specified in the platform status", + controller: &albo.AWSLoadBalancerController{ + Spec: albo.AWSLoadBalancerControllerSpec{}, + }, + platformStatus: &configv1.PlatformStatus{ + Type: configv1.AWSPlatformType, + AWS: &configv1.AWSPlatformStatus{ + ResourceTags: []configv1.AWSResourceTag{ + {Key: "key1", Value: "value1"}, + {Key: "key2", Value: "value2"}, + }, + }, + }, + expectedArgs: sets.New[string]( + "--enable-shield=false", + "--enable-waf=false", + "--enable-wafv2=false", + "--ingress-class=alb", + "--default-tags=key1=value1,key2=value2", + ), + }, + { + name: "conflicting resource tags specified in the platform status and operator spec", + controller: &albo.AWSLoadBalancerController{ + Spec: albo.AWSLoadBalancerControllerSpec{ + AdditionalResourceTags: []albo.AWSResourceTag{ + {Key: "op-key1", Value: "op-value1"}, + {Key: "conflict-key1", Value: "op-value2"}, + {Key: "conflict-key2", Value: "op-value3"}, + }, + }, + }, + platformStatus: &configv1.PlatformStatus{ + Type: configv1.AWSPlatformType, + AWS: &configv1.AWSPlatformStatus{ + ResourceTags: []configv1.AWSResourceTag{ + {Key: "plat-key1", Value: "plat-value1"}, + {Key: "conflict-key1", Value: "plat-value2"}, + {Key: "conflict-key2", Value: "plat-value3"}, + }, + }, + }, + expectedArgs: sets.New[string]( + "--enable-shield=false", + "--enable-waf=false", + "--enable-wafv2=false", + "--ingress-class=alb", + "--default-tags=conflict-key1=op-value2,conflict-key2=op-value3,op-key1=op-value1,plat-key1=plat-value1", + ), + }, + { + name: "non-conflicting resource tags specified in the platform status and operator spec", + controller: &albo.AWSLoadBalancerController{ + Spec: albo.AWSLoadBalancerControllerSpec{ + AdditionalResourceTags: []albo.AWSResourceTag{ + {Key: "op-key1", Value: "op-value1"}, + {Key: "op-key2", Value: "op-value2"}, + }, + }, + }, + platformStatus: &configv1.PlatformStatus{ + Type: configv1.AWSPlatformType, + AWS: &configv1.AWSPlatformStatus{ + ResourceTags: []configv1.AWSResourceTag{ + {Key: "plat-key1", Value: "plat-value1"}, + {Key: "plat-key2", Value: "plat-value2"}, + }, + }, + }, + expectedArgs: sets.New[string]( + "--enable-shield=false", + "--enable-waf=false", + "--enable-wafv2=false", + "--ingress-class=alb", + "--default-tags=op-key1=op-value1,op-key2=op-value2,plat-key1=plat-value1,plat-key2=plat-value2", + ), + }, + { + name: "when merged tags exceeded maximum of 24 allowed tags", + controller: &albo.AWSLoadBalancerController{ + Spec: albo.AWSLoadBalancerControllerSpec{ + AdditionalResourceTags: []albo.AWSResourceTag{}, + }, + }, + platformStatus: &configv1.PlatformStatus{ + Type: configv1.AWSPlatformType, + AWS: &configv1.AWSPlatformStatus{ + ResourceTags: generateAWSResourceTags(25), + }, + }, + expectedError: true, + }, } { t.Run(tc.name, func(t *testing.T) { defaultArgs := sets.New[string]( @@ -142,12 +239,19 @@ func TestDesiredArgs(t *testing.T) { if tc.controller.Spec.IngressClass == "" { tc.controller.Spec.IngressClass = "alb" } - args := desiredContainerArgs(tc.controller, "test-cluster", "test-vpc") - - expected := sets.List(expectedArgs) - sort.Strings(expected) - if diff := cmp.Diff(expected, args); diff != "" { - t.Fatalf("unexpected arguments\n%s", diff) + if tc.platformStatus == nil { + tc.platformStatus = &configv1.PlatformStatus{} + } + args, gotErr := desiredContainerArgs(tc.controller, tc.platformStatus, "test-cluster", "test-vpc") + if (gotErr != nil) != tc.expectedError { + t.Fatalf("expected errors to be %t, but got %t", tc.expectedError, gotErr != nil) + } + if !tc.expectedError { + expected := sets.List(expectedArgs) + sort.Strings(expected) + if diff := cmp.Diff(expected, args); diff != "" { + t.Fatalf("unexpected arguments\n%s", diff) + } } }) } @@ -743,11 +847,15 @@ func TestEnsureDeployment(t *testing.T) { VPCID: "test-vpc", AWSRegion: testAWSRegion, } - _, err := r.ensureDeployment(context.Background(), tc.serviceAccount, "test-credentials", "test-serving", tc.controller, tc.trustedCAConfigMap) + _, err := r.ensureDeployment(context.Background(), tc.serviceAccount, "test-credentials", "test-serving", tc.controller, &configv1.PlatformStatus{}, tc.trustedCAConfigMap) if err != nil { t.Fatalf("unexpected error: %v", err) } - tc.expectedDeployment.Spec.Template.Spec.Containers[0].Args = desiredContainerArgs(tc.controller, "test-cluster", "test-vpc") + args, err := desiredContainerArgs(tc.controller, &configv1.PlatformStatus{}, "test-cluster", "test-vpc") + if err != nil { + t.Fatalf("failed to get container args: %v", err) + } + tc.expectedDeployment.Spec.Template.Spec.Containers[0].Args = args var deployment appsv1.Deployment err = client.Get(context.Background(), types.NamespacedName{Namespace: "test-namespace", Name: fmt.Sprintf("%s-%s", controllerResourcePrefix, tc.controller.Name)}, &deployment) if err != nil { @@ -854,11 +962,15 @@ func TestEnsureDeploymentEnvVars(t *testing.T) { VPCID: "test-vpc", AWSRegion: testAWSRegion, } - _, err := r.ensureDeployment(context.Background(), tc.serviceAccount, "test-credentials", "test-serving", tc.controller, nil) + _, err := r.ensureDeployment(context.Background(), tc.serviceAccount, "test-credentials", "test-serving", tc.controller, &configv1.PlatformStatus{}, nil) if err != nil { t.Fatalf("unexpected error: %v", err) } - tc.expectedDeployment.Spec.Template.Spec.Containers[0].Args = desiredContainerArgs(tc.controller, "test-cluster", "test-vpc") + args, err := desiredContainerArgs(tc.controller, &configv1.PlatformStatus{}, "test-cluster", "test-vpc") + if err != nil { + t.Fatalf("failed to get container args: %v", err) + } + tc.expectedDeployment.Spec.Template.Spec.Containers[0].Args = args var deployment appsv1.Deployment err = client.Get(context.Background(), types.NamespacedName{Namespace: "test-namespace", Name: fmt.Sprintf("%s-%s", controllerResourcePrefix, tc.controller.Name)}, &deployment) if err != nil { @@ -1233,3 +1345,15 @@ func (b *testContainerBuilder) build() corev1.Container { SecurityContext: b.securityContext, } } + +// generateAWSResourceTags generates AWSResourceTags with a length of `n`. +func generateAWSResourceTags(n int) []configv1.AWSResourceTag { + tags := make([]configv1.AWSResourceTag, n) + for i := 0; i < n; i++ { + tags[i] = configv1.AWSResourceTag{ + Key: fmt.Sprintf("key-%d", i), + Value: fmt.Sprintf("value-%d", i), + } + } + return tags +} diff --git a/pkg/controllers/suite_test.go b/pkg/controllers/suite_test.go index 41706610..3ef9079f 100644 --- a/pkg/controllers/suite_test.go +++ b/pkg/controllers/suite_test.go @@ -35,6 +35,8 @@ import ( logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/log/zap" + configv1 "github.com/openshift/api/config/v1" + albo "github.com/openshift/aws-load-balancer-operator/api/v1" albc "github.com/openshift/aws-load-balancer-operator/pkg/controllers/awsloadbalancercontroller" //+kubebuilder:scaffold:imports @@ -82,6 +84,9 @@ var _ = BeforeSuite(func() { err = cco.Install(scheme.Scheme) Expect(err).NotTo(HaveOccurred()) + err = configv1.Install(scheme.Scheme) + Expect(err).NotTo(HaveOccurred()) + //+kubebuilder:scaffold:scheme k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme}) diff --git a/pkg/controllers/watch_test.go b/pkg/controllers/watch_test.go index beeaaa7a..73873a92 100644 --- a/pkg/controllers/watch_test.go +++ b/pkg/controllers/watch_test.go @@ -8,6 +8,7 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + configv1 "github.com/openshift/api/config/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -98,6 +99,38 @@ var _ = Describe("AWS Load Balancer Reconciler Watch Predicates", func() { Expect(gotReq).To(Equal(expectedReq)) }) }) + + Context("Infrastructure", func() { + It("does not trigger reconciliation on 'wrong' Infrastructure changes", func() { + infra := &configv1.Infrastructure{ + ObjectMeta: metav1.ObjectMeta{ + Name: "wrong", + }, + } + var gotReq ctrl.Request + errCh := waitForRequest(reconcileCollector.Requests, 1*time.Second, &gotReq) + Expect(k8sClient.Create(context.Background(), infra)).Should(Succeed()) + Expect(<-errCh).NotTo(BeNil()) + }) + It("triggers reconciliation on 'cluster' Infrastructure changes", func() { + infra := &configv1.Infrastructure{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster", + }, + } + expectedReq := ctrl.Request{ + NamespacedName: types.NamespacedName{ + Name: "cluster", + }, + } + var gotReq ctrl.Request + errCh := waitForRequest(reconcileCollector.Requests, 1*time.Second, &gotReq) + Expect(k8sClient.Create(context.Background(), infra)).Should(Succeed()) + Expect(<-errCh).To(BeNil()) + Expect(gotReq).To(Equal(expectedReq)) + }) + + }) }) func waitForRequest(requests chan ctrl.Request, timeout time.Duration, res *ctrl.Request) <-chan error { diff --git a/pkg/utils/test/crd/infrastructure-Default.yaml b/pkg/utils/test/crd/infrastructure-Default.yaml new file mode 100644 index 00000000..64a54d5c --- /dev/null +++ b/pkg/utils/test/crd/infrastructure-Default.yaml @@ -0,0 +1,855 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + api-approved.openshift.io: https://github.com/openshift/api/pull/470 + include.release.openshift.io/ibm-cloud-managed: "true" + include.release.openshift.io/self-managed-high-availability: "true" + include.release.openshift.io/single-node-developer: "true" + release.openshift.io/feature-set: Default + name: infrastructures.config.openshift.io +spec: + group: config.openshift.io + names: + kind: Infrastructure + listKind: InfrastructureList + plural: infrastructures + singular: infrastructure + scope: Cluster + versions: + - name: v1 + schema: + openAPIV3Schema: + description: "Infrastructure holds cluster-wide information about Infrastructure. The canonical name is `cluster` \n Compatibility level 1: Stable within a major release for a minimum of 12 months or 3 minor releases (whichever is longer)." + properties: + apiVersion: + description: 'APIVersion defines the versioned schema of this representation of an object. Servers should convert recognized schemas to the latest internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' + type: string + kind: + description: 'Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + type: string + metadata: + type: object + spec: + description: spec holds user settable values for configuration + properties: + cloudConfig: + description: "cloudConfig is a reference to a ConfigMap containing the cloud provider configuration file. This configuration file is used to configure the Kubernetes cloud provider integration when using the built-in cloud provider integration or the external cloud controller manager. The namespace for this config map is openshift-config. \n cloudConfig should only be consumed by the kube_cloud_config controller. The controller is responsible for using the user configuration in the spec for various platforms and combining that with the user provided ConfigMap in this field to create a stitched kube cloud config. The controller generates a ConfigMap `kube-cloud-config` in `openshift-config-managed` namespace with the kube cloud config is stored in `cloud.conf` key. All the clients are expected to use the generated ConfigMap only." + properties: + key: + description: Key allows pointing to a specific key/value inside of the configmap. This is useful for logical file references. + type: string + name: + type: string + type: object + platformSpec: + description: platformSpec holds desired information specific to the underlying infrastructure provider. + properties: + alibabaCloud: + description: AlibabaCloud contains settings specific to the Alibaba Cloud infrastructure provider. + type: object + aws: + description: AWS contains settings specific to the Amazon Web Services infrastructure provider. + properties: + serviceEndpoints: + description: serviceEndpoints list contains custom endpoints which will override default service endpoint of AWS Services. There must be only one ServiceEndpoint for a service. + items: + description: AWSServiceEndpoint store the configuration of a custom url to override existing defaults of AWS Services. + properties: + name: + description: name is the name of the AWS service. The list of all the service names can be found at https://docs.aws.amazon.com/general/latest/gr/aws-service-information.html This must be provided and cannot be empty. + pattern: ^[a-z0-9-]+$ + type: string + url: + description: url is fully qualified URI with scheme https, that overrides the default generated endpoint for a client. This must be provided and cannot be empty. + pattern: ^https:// + type: string + type: object + type: array + type: object + azure: + description: Azure contains settings specific to the Azure infrastructure provider. + type: object + baremetal: + description: BareMetal contains settings specific to the BareMetal platform. + type: object + equinixMetal: + description: EquinixMetal contains settings specific to the Equinix Metal infrastructure provider. + type: object + external: + description: ExternalPlatformType represents generic infrastructure provider. Platform-specific components should be supplemented separately. + properties: + platformName: + default: Unknown + description: PlatformName holds the arbitrary string representing the infrastructure provider name, expected to be set at the installation time. This field is solely for informational and reporting purposes and is not expected to be used for decision-making. + type: string + x-kubernetes-validations: + - message: platform name cannot be changed once set + rule: oldSelf == 'Unknown' || self == oldSelf + type: object + gcp: + description: GCP contains settings specific to the Google Cloud Platform infrastructure provider. + type: object + ibmcloud: + description: IBMCloud contains settings specific to the IBMCloud infrastructure provider. + type: object + kubevirt: + description: Kubevirt contains settings specific to the kubevirt infrastructure provider. + type: object + nutanix: + description: Nutanix contains settings specific to the Nutanix infrastructure provider. + properties: + prismCentral: + description: prismCentral holds the endpoint address and port to access the Nutanix Prism Central. When a cluster-wide proxy is installed, by default, this endpoint will be accessed via the proxy. Should you wish for communication with this endpoint not to be proxied, please add the endpoint to the proxy spec.noProxy list. + properties: + address: + description: address is the endpoint address (DNS name or IP address) of the Nutanix Prism Central or Element (cluster) + maxLength: 256 + type: string + port: + description: port is the port number to access the Nutanix Prism Central or Element (cluster) + format: int32 + maximum: 65535 + minimum: 1 + type: integer + required: + - address + - port + type: object + prismElements: + description: prismElements holds one or more endpoint address and port data to access the Nutanix Prism Elements (clusters) of the Nutanix Prism Central. Currently we only support one Prism Element (cluster) for an OpenShift cluster, where all the Nutanix resources (VMs, subnets, volumes, etc.) used in the OpenShift cluster are located. In the future, we may support Nutanix resources (VMs, etc.) spread over multiple Prism Elements (clusters) of the Prism Central. + items: + description: NutanixPrismElementEndpoint holds the name and endpoint data for a Prism Element (cluster) + properties: + endpoint: + description: endpoint holds the endpoint address and port data of the Prism Element (cluster). When a cluster-wide proxy is installed, by default, this endpoint will be accessed via the proxy. Should you wish for communication with this endpoint not to be proxied, please add the endpoint to the proxy spec.noProxy list. + properties: + address: + description: address is the endpoint address (DNS name or IP address) of the Nutanix Prism Central or Element (cluster) + maxLength: 256 + type: string + port: + description: port is the port number to access the Nutanix Prism Central or Element (cluster) + format: int32 + maximum: 65535 + minimum: 1 + type: integer + required: + - address + - port + type: object + name: + description: name is the name of the Prism Element (cluster). This value will correspond with the cluster field configured on other resources (eg Machines, PVCs, etc). + maxLength: 256 + type: string + required: + - endpoint + - name + type: object + type: array + x-kubernetes-list-map-keys: + - name + x-kubernetes-list-type: map + required: + - prismCentral + - prismElements + type: object + openstack: + description: OpenStack contains settings specific to the OpenStack infrastructure provider. + type: object + ovirt: + description: Ovirt contains settings specific to the oVirt infrastructure provider. + type: object + powervs: + description: PowerVS contains settings specific to the IBM Power Systems Virtual Servers infrastructure provider. + properties: + serviceEndpoints: + description: serviceEndpoints is a list of custom endpoints which will override the default service endpoints of a Power VS service. + items: + description: PowervsServiceEndpoint stores the configuration of a custom url to override existing defaults of PowerVS Services. + properties: + name: + description: name is the name of the Power VS service. Few of the services are IAM - https://cloud.ibm.com/apidocs/iam-identity-token-api ResourceController - https://cloud.ibm.com/apidocs/resource-controller/resource-controller Power Cloud - https://cloud.ibm.com/apidocs/power-cloud + pattern: ^[a-z0-9-]+$ + type: string + url: + description: url is fully qualified URI with scheme https, that overrides the default generated endpoint for a client. This must be provided and cannot be empty. + format: uri + pattern: ^https:// + type: string + required: + - name + - url + type: object + type: array + x-kubernetes-list-map-keys: + - name + x-kubernetes-list-type: map + type: object + type: + description: type is the underlying infrastructure provider for the cluster. This value controls whether infrastructure automation such as service load balancers, dynamic volume provisioning, machine creation and deletion, and other integrations are enabled. If None, no infrastructure automation is enabled. Allowed values are "AWS", "Azure", "BareMetal", "GCP", "Libvirt", "OpenStack", "VSphere", "oVirt", "KubeVirt", "EquinixMetal", "PowerVS", "AlibabaCloud", "Nutanix" and "None". Individual components may not support all platforms, and must handle unrecognized platforms as None if they do not support that platform. + enum: + - "" + - AWS + - Azure + - BareMetal + - GCP + - Libvirt + - OpenStack + - None + - VSphere + - oVirt + - IBMCloud + - KubeVirt + - EquinixMetal + - PowerVS + - AlibabaCloud + - Nutanix + - External + type: string + vsphere: + description: VSphere contains settings specific to the VSphere infrastructure provider. + properties: + failureDomains: + description: failureDomains contains the definition of region, zone and the vCenter topology. If this is omitted failure domains (regions and zones) will not be used. + items: + description: VSpherePlatformFailureDomainSpec holds the region and zone failure domain and the vCenter topology of that failure domain. + properties: + name: + description: name defines the arbitrary but unique name of a failure domain. + maxLength: 256 + minLength: 1 + type: string + region: + description: region defines the name of a region tag that will be attached to a vCenter datacenter. The tag category in vCenter must be named openshift-region. + maxLength: 80 + minLength: 1 + type: string + server: + anyOf: + - format: ipv4 + - format: ipv6 + - format: hostname + description: server is the fully-qualified domain name or the IP address of the vCenter server. --- + maxLength: 255 + minLength: 1 + type: string + topology: + description: Topology describes a given failure domain using vSphere constructs + properties: + computeCluster: + description: computeCluster the absolute path of the vCenter cluster in which virtual machine will be located. The absolute path is of the form //host/. The maximum length of the path is 2048 characters. + maxLength: 2048 + pattern: ^/.*?/host/.*? + type: string + datacenter: + description: datacenter is the name of vCenter datacenter in which virtual machines will be located. The maximum length of the datacenter name is 80 characters. + maxLength: 80 + type: string + datastore: + description: datastore is the absolute path of the datastore in which the virtual machine is located. The absolute path is of the form //datastore/ The maximum length of the path is 2048 characters. + maxLength: 2048 + pattern: ^/.*?/datastore/.*? + type: string + folder: + description: folder is the absolute path of the folder where virtual machines are located. The absolute path is of the form //vm/. The maximum length of the path is 2048 characters. + maxLength: 2048 + pattern: ^/.*?/vm/.*? + type: string + networks: + description: networks is the list of port group network names within this failure domain. Currently, we only support a single interface per RHCOS virtual machine. The available networks (port groups) can be listed using `govc ls 'network/*'` The single interface should be the absolute path of the form //network/. + items: + type: string + maxItems: 1 + minItems: 1 + type: array + resourcePool: + description: resourcePool is the absolute path of the resource pool where virtual machines will be created. The absolute path is of the form //host//Resources/. The maximum length of the path is 2048 characters. + maxLength: 2048 + pattern: ^/.*?/host/.*?/Resources.* + type: string + required: + - computeCluster + - datacenter + - datastore + - networks + type: object + zone: + description: zone defines the name of a zone tag that will be attached to a vCenter cluster. The tag category in vCenter must be named openshift-zone. + maxLength: 80 + minLength: 1 + type: string + required: + - name + - region + - server + - topology + - zone + type: object + type: array + nodeNetworking: + description: nodeNetworking contains the definition of internal and external network constraints for assigning the node's networking. If this field is omitted, networking defaults to the legacy address selection behavior which is to only support a single address and return the first one found. + properties: + external: + description: external represents the network configuration of the node that is externally routable. + properties: + excludeNetworkSubnetCidr: + description: excludeNetworkSubnetCidr IP addresses in subnet ranges will be excluded when selecting the IP address from the VirtualMachine's VM for use in the status.addresses fields. --- + items: + format: cidr + type: string + type: array + network: + description: network VirtualMachine's VM Network names that will be used to when searching for status.addresses fields. Note that if internal.networkSubnetCIDR and external.networkSubnetCIDR are not set, then the vNIC associated to this network must only have a single IP address assigned to it. The available networks (port groups) can be listed using `govc ls 'network/*'` + type: string + networkSubnetCidr: + description: networkSubnetCidr IP address on VirtualMachine's network interfaces included in the fields' CIDRs that will be used in respective status.addresses fields. --- + items: + format: cidr + type: string + type: array + type: object + internal: + description: internal represents the network configuration of the node that is routable only within the cluster. + properties: + excludeNetworkSubnetCidr: + description: excludeNetworkSubnetCidr IP addresses in subnet ranges will be excluded when selecting the IP address from the VirtualMachine's VM for use in the status.addresses fields. --- + items: + format: cidr + type: string + type: array + network: + description: network VirtualMachine's VM Network names that will be used to when searching for status.addresses fields. Note that if internal.networkSubnetCIDR and external.networkSubnetCIDR are not set, then the vNIC associated to this network must only have a single IP address assigned to it. The available networks (port groups) can be listed using `govc ls 'network/*'` + type: string + networkSubnetCidr: + description: networkSubnetCidr IP address on VirtualMachine's network interfaces included in the fields' CIDRs that will be used in respective status.addresses fields. --- + items: + format: cidr + type: string + type: array + type: object + type: object + vcenters: + description: vcenters holds the connection details for services to communicate with vCenter. Currently, only a single vCenter is supported. --- + items: + description: VSpherePlatformVCenterSpec stores the vCenter connection fields. This is used by the vSphere CCM. + properties: + datacenters: + description: The vCenter Datacenters in which the RHCOS vm guests are located. This field will be used by the Cloud Controller Manager. Each datacenter listed here should be used within a topology. + items: + type: string + minItems: 1 + type: array + port: + description: port is the TCP port that will be used to communicate to the vCenter endpoint. When omitted, this means the user has no opinion and it is up to the platform to choose a sensible default, which is subject to change over time. + format: int32 + maximum: 32767 + minimum: 1 + type: integer + server: + anyOf: + - format: ipv4 + - format: ipv6 + - format: hostname + description: server is the fully-qualified domain name or the IP address of the vCenter server. --- + maxLength: 255 + type: string + required: + - datacenters + - server + type: object + maxItems: 1 + minItems: 0 + type: array + type: object + type: object + type: object + status: + description: status holds observed values from the cluster. They may not be overridden. + properties: + apiServerInternalURI: + description: apiServerInternalURL is a valid URI with scheme 'https', address and optionally a port (defaulting to 443). apiServerInternalURL can be used by components like kubelets, to contact the Kubernetes API server using the infrastructure provider rather than Kubernetes networking. + type: string + apiServerURL: + description: apiServerURL is a valid URI with scheme 'https', address and optionally a port (defaulting to 443). apiServerURL can be used by components like the web console to tell users where to find the Kubernetes API. + type: string + controlPlaneTopology: + default: HighlyAvailable + description: controlPlaneTopology expresses the expectations for operands that normally run on control nodes. The default is 'HighlyAvailable', which represents the behavior operators have in a "normal" cluster. The 'SingleReplica' mode will be used in single-node deployments and the operators should not configure the operand for highly-available operation The 'External' mode indicates that the control plane is hosted externally to the cluster and that its components are not visible within the cluster. + enum: + - HighlyAvailable + - SingleReplica + - External + type: string + etcdDiscoveryDomain: + description: 'etcdDiscoveryDomain is the domain used to fetch the SRV records for discovering etcd servers and clients. For more info: https://github.com/etcd-io/etcd/blob/329be66e8b3f9e2e6af83c123ff89297e49ebd15/Documentation/op-guide/clustering.md#dns-discovery deprecated: as of 4.7, this field is no longer set or honored. It will be removed in a future release.' + type: string + infrastructureName: + description: infrastructureName uniquely identifies a cluster with a human friendly name. Once set it should not be changed. Must be of max length 27 and must have only alphanumeric or hyphen characters. + type: string + infrastructureTopology: + default: HighlyAvailable + description: 'infrastructureTopology expresses the expectations for infrastructure services that do not run on control plane nodes, usually indicated by a node selector for a `role` value other than `master`. The default is ''HighlyAvailable'', which represents the behavior operators have in a "normal" cluster. The ''SingleReplica'' mode will be used in single-node deployments and the operators should not configure the operand for highly-available operation NOTE: External topology mode is not applicable for this field.' + enum: + - HighlyAvailable + - SingleReplica + type: string + platform: + description: "platform is the underlying infrastructure provider for the cluster. \n Deprecated: Use platformStatus.type instead." + enum: + - "" + - AWS + - Azure + - BareMetal + - GCP + - Libvirt + - OpenStack + - None + - VSphere + - oVirt + - IBMCloud + - KubeVirt + - EquinixMetal + - PowerVS + - AlibabaCloud + - Nutanix + - External + type: string + platformStatus: + description: platformStatus holds status information specific to the underlying infrastructure provider. + properties: + alibabaCloud: + description: AlibabaCloud contains settings specific to the Alibaba Cloud infrastructure provider. + properties: + region: + description: region specifies the region for Alibaba Cloud resources created for the cluster. + pattern: ^[0-9A-Za-z-]+$ + type: string + resourceGroupID: + description: resourceGroupID is the ID of the resource group for the cluster. + pattern: ^(rg-[0-9A-Za-z]+)?$ + type: string + resourceTags: + description: resourceTags is a list of additional tags to apply to Alibaba Cloud resources created for the cluster. + items: + description: AlibabaCloudResourceTag is the set of tags to add to apply to resources. + properties: + key: + description: key is the key of the tag. + maxLength: 128 + minLength: 1 + type: string + value: + description: value is the value of the tag. + maxLength: 128 + minLength: 1 + type: string + required: + - key + - value + type: object + maxItems: 20 + type: array + x-kubernetes-list-map-keys: + - key + x-kubernetes-list-type: map + required: + - region + type: object + aws: + description: AWS contains settings specific to the Amazon Web Services infrastructure provider. + properties: + region: + description: region holds the default AWS region for new AWS resources created by the cluster. + type: string + resourceTags: + description: resourceTags is a list of additional tags to apply to AWS resources created for the cluster. See https://docs.aws.amazon.com/general/latest/gr/aws_tagging.html for information on tagging AWS resources. AWS supports a maximum of 50 tags per resource. OpenShift reserves 25 tags for its use, leaving 25 tags available for the user. + items: + description: AWSResourceTag is a tag to apply to AWS resources created for the cluster. + properties: + key: + description: key is the key of the tag + maxLength: 128 + minLength: 1 + pattern: ^[0-9A-Za-z_.:/=+-@]+$ + type: string + value: + description: value is the value of the tag. Some AWS service do not support empty values. Since tags are added to resources in many services, the length of the tag value must meet the requirements of all services. + maxLength: 256 + minLength: 1 + pattern: ^[0-9A-Za-z_.:/=+-@]+$ + type: string + required: + - key + - value + type: object + maxItems: 25 + type: array + serviceEndpoints: + description: ServiceEndpoints list contains custom endpoints which will override default service endpoint of AWS Services. There must be only one ServiceEndpoint for a service. + items: + description: AWSServiceEndpoint store the configuration of a custom url to override existing defaults of AWS Services. + properties: + name: + description: name is the name of the AWS service. The list of all the service names can be found at https://docs.aws.amazon.com/general/latest/gr/aws-service-information.html This must be provided and cannot be empty. + pattern: ^[a-z0-9-]+$ + type: string + url: + description: url is fully qualified URI with scheme https, that overrides the default generated endpoint for a client. This must be provided and cannot be empty. + pattern: ^https:// + type: string + type: object + type: array + type: object + azure: + description: Azure contains settings specific to the Azure infrastructure provider. + properties: + armEndpoint: + description: armEndpoint specifies a URL to use for resource management in non-soverign clouds such as Azure Stack. + type: string + cloudName: + description: cloudName is the name of the Azure cloud environment which can be used to configure the Azure SDK with the appropriate Azure API endpoints. If empty, the value is equal to `AzurePublicCloud`. + enum: + - "" + - AzurePublicCloud + - AzureUSGovernmentCloud + - AzureChinaCloud + - AzureGermanCloud + - AzureStackCloud + type: string + networkResourceGroupName: + description: networkResourceGroupName is the Resource Group for network resources like the Virtual Network and Subnets used by the cluster. If empty, the value is same as ResourceGroupName. + type: string + resourceGroupName: + description: resourceGroupName is the Resource Group for new Azure resources created for the cluster. + type: string + resourceTags: + description: resourceTags is a list of additional tags to apply to Azure resources created for the cluster. See https://docs.microsoft.com/en-us/rest/api/resources/tags for information on tagging Azure resources. Due to limitations on Automation, Content Delivery Network, DNS Azure resources, a maximum of 15 tags may be applied. OpenShift reserves 5 tags for internal use, allowing 10 tags for user configuration. + items: + description: AzureResourceTag is a tag to apply to Azure resources created for the cluster. + properties: + key: + description: key is the key part of the tag. A tag key can have a maximum of 128 characters and cannot be empty. Key must begin with a letter, end with a letter, number or underscore, and must contain only alphanumeric characters and the following special characters `_ . -`. + maxLength: 128 + minLength: 1 + pattern: ^[a-zA-Z]([0-9A-Za-z_.-]*[0-9A-Za-z_])?$ + type: string + value: + description: 'value is the value part of the tag. A tag value can have a maximum of 256 characters and cannot be empty. Value must contain only alphanumeric characters and the following special characters `_ + , - . / : ; < = > ? @`.' + maxLength: 256 + minLength: 1 + pattern: ^[0-9A-Za-z_.=+-@]+$ + type: string + required: + - key + - value + type: object + maxItems: 10 + type: array + x-kubernetes-validations: + - message: resourceTags are immutable and may only be configured during installation + rule: self.all(x, x in oldSelf) && oldSelf.all(x, x in self) + type: object + x-kubernetes-validations: + - message: resourceTags may only be configured during installation + rule: '!has(oldSelf.resourceTags) && !has(self.resourceTags) || has(oldSelf.resourceTags) && has(self.resourceTags)' + baremetal: + description: BareMetal contains settings specific to the BareMetal platform. + properties: + apiServerInternalIP: + description: "apiServerInternalIP is an IP address to contact the Kubernetes API server that can be used by components inside the cluster, like kubelets using the infrastructure rather than Kubernetes networking. It is the IP that the Infrastructure.status.apiServerInternalURI points to. It is the IP for a self-hosted load balancer in front of the API servers. \n Deprecated: Use APIServerInternalIPs instead." + type: string + apiServerInternalIPs: + description: apiServerInternalIPs are the IP addresses to contact the Kubernetes API server that can be used by components inside the cluster, like kubelets using the infrastructure rather than Kubernetes networking. These are the IPs for a self-hosted load balancer in front of the API servers. In dual stack clusters this list contains two IPs otherwise only one. + format: ip + items: + type: string + maxItems: 2 + type: array + ingressIP: + description: "ingressIP is an external IP which routes to the default ingress controller. The IP is a suitable target of a wildcard DNS record used to resolve default route host names. \n Deprecated: Use IngressIPs instead." + type: string + ingressIPs: + description: ingressIPs are the external IPs which route to the default ingress controller. The IPs are suitable targets of a wildcard DNS record used to resolve default route host names. In dual stack clusters this list contains two IPs otherwise only one. + format: ip + items: + type: string + maxItems: 2 + type: array + nodeDNSIP: + description: nodeDNSIP is the IP address for the internal DNS used by the nodes. Unlike the one managed by the DNS operator, `NodeDNSIP` provides name resolution for the nodes themselves. There is no DNS-as-a-service for BareMetal deployments. In order to minimize necessary changes to the datacenter DNS, a DNS service is hosted as a static pod to serve those hostnames to the nodes in the cluster. + type: string + type: object + equinixMetal: + description: EquinixMetal contains settings specific to the Equinix Metal infrastructure provider. + properties: + apiServerInternalIP: + description: apiServerInternalIP is an IP address to contact the Kubernetes API server that can be used by components inside the cluster, like kubelets using the infrastructure rather than Kubernetes networking. It is the IP that the Infrastructure.status.apiServerInternalURI points to. It is the IP for a self-hosted load balancer in front of the API servers. + type: string + ingressIP: + description: ingressIP is an external IP which routes to the default ingress controller. The IP is a suitable target of a wildcard DNS record used to resolve default route host names. + type: string + type: object + external: + description: External contains settings specific to the generic External infrastructure provider. + properties: + cloudControllerManager: + description: cloudControllerManager contains settings specific to the external Cloud Controller Manager (a.k.a. CCM or CPI). When omitted, new nodes will be not tainted and no extra initialization from the cloud controller manager is expected. + properties: + state: + description: "state determines whether or not an external Cloud Controller Manager is expected to be installed within the cluster. https://kubernetes.io/docs/tasks/administer-cluster/running-cloud-controller/#running-cloud-controller-manager \n Valid values are \"External\", \"None\" and omitted. When set to \"External\", new nodes will be tainted as uninitialized when created, preventing them from running workloads until they are initialized by the cloud controller manager. When omitted or set to \"None\", new nodes will be not tainted and no extra initialization from the cloud controller manager is expected." + enum: + - "" + - External + - None + type: string + x-kubernetes-validations: + - message: state is immutable once set + rule: self == oldSelf + type: object + x-kubernetes-validations: + - message: state may not be added or removed once set + rule: (has(self.state) == has(oldSelf.state)) || (!has(oldSelf.state) && self.state != "External") + type: object + x-kubernetes-validations: + - message: cloudControllerManager may not be added or removed once set + rule: has(self.cloudControllerManager) == has(oldSelf.cloudControllerManager) + gcp: + description: GCP contains settings specific to the Google Cloud Platform infrastructure provider. + properties: + projectID: + description: resourceGroupName is the Project ID for new GCP resources created for the cluster. + type: string + region: + description: region holds the region for new GCP resources created for the cluster. + type: string + type: object + ibmcloud: + description: IBMCloud contains settings specific to the IBMCloud infrastructure provider. + properties: + cisInstanceCRN: + description: CISInstanceCRN is the CRN of the Cloud Internet Services instance managing the DNS zone for the cluster's base domain + type: string + dnsInstanceCRN: + description: DNSInstanceCRN is the CRN of the DNS Services instance managing the DNS zone for the cluster's base domain + type: string + location: + description: Location is where the cluster has been deployed + type: string + providerType: + description: ProviderType indicates the type of cluster that was created + type: string + resourceGroupName: + description: ResourceGroupName is the Resource Group for new IBMCloud resources created for the cluster. + type: string + type: object + kubevirt: + description: Kubevirt contains settings specific to the kubevirt infrastructure provider. + properties: + apiServerInternalIP: + description: apiServerInternalIP is an IP address to contact the Kubernetes API server that can be used by components inside the cluster, like kubelets using the infrastructure rather than Kubernetes networking. It is the IP that the Infrastructure.status.apiServerInternalURI points to. It is the IP for a self-hosted load balancer in front of the API servers. + type: string + ingressIP: + description: ingressIP is an external IP which routes to the default ingress controller. The IP is a suitable target of a wildcard DNS record used to resolve default route host names. + type: string + type: object + nutanix: + description: Nutanix contains settings specific to the Nutanix infrastructure provider. + properties: + apiServerInternalIP: + description: "apiServerInternalIP is an IP address to contact the Kubernetes API server that can be used by components inside the cluster, like kubelets using the infrastructure rather than Kubernetes networking. It is the IP that the Infrastructure.status.apiServerInternalURI points to. It is the IP for a self-hosted load balancer in front of the API servers. \n Deprecated: Use APIServerInternalIPs instead." + type: string + apiServerInternalIPs: + description: apiServerInternalIPs are the IP addresses to contact the Kubernetes API server that can be used by components inside the cluster, like kubelets using the infrastructure rather than Kubernetes networking. These are the IPs for a self-hosted load balancer in front of the API servers. In dual stack clusters this list contains two IPs otherwise only one. + format: ip + items: + type: string + maxItems: 2 + type: array + ingressIP: + description: "ingressIP is an external IP which routes to the default ingress controller. The IP is a suitable target of a wildcard DNS record used to resolve default route host names. \n Deprecated: Use IngressIPs instead." + type: string + ingressIPs: + description: ingressIPs are the external IPs which route to the default ingress controller. The IPs are suitable targets of a wildcard DNS record used to resolve default route host names. In dual stack clusters this list contains two IPs otherwise only one. + format: ip + items: + type: string + maxItems: 2 + type: array + type: object + openstack: + description: OpenStack contains settings specific to the OpenStack infrastructure provider. + properties: + apiServerInternalIP: + description: "apiServerInternalIP is an IP address to contact the Kubernetes API server that can be used by components inside the cluster, like kubelets using the infrastructure rather than Kubernetes networking. It is the IP that the Infrastructure.status.apiServerInternalURI points to. It is the IP for a self-hosted load balancer in front of the API servers. \n Deprecated: Use APIServerInternalIPs instead." + type: string + apiServerInternalIPs: + description: apiServerInternalIPs are the IP addresses to contact the Kubernetes API server that can be used by components inside the cluster, like kubelets using the infrastructure rather than Kubernetes networking. These are the IPs for a self-hosted load balancer in front of the API servers. In dual stack clusters this list contains two IPs otherwise only one. + format: ip + items: + type: string + maxItems: 2 + type: array + cloudName: + description: cloudName is the name of the desired OpenStack cloud in the client configuration file (`clouds.yaml`). + type: string + ingressIP: + description: "ingressIP is an external IP which routes to the default ingress controller. The IP is a suitable target of a wildcard DNS record used to resolve default route host names. \n Deprecated: Use IngressIPs instead." + type: string + ingressIPs: + description: ingressIPs are the external IPs which route to the default ingress controller. The IPs are suitable targets of a wildcard DNS record used to resolve default route host names. In dual stack clusters this list contains two IPs otherwise only one. + format: ip + items: + type: string + maxItems: 2 + type: array + loadBalancer: + default: + type: OpenShiftManagedDefault + description: loadBalancer defines how the load balancer used by the cluster is configured. + properties: + type: + default: OpenShiftManagedDefault + description: type defines the type of load balancer used by the cluster on OpenStack platform which can be a user-managed or openshift-managed load balancer that is to be used for the OpenShift API and Ingress endpoints. When set to OpenShiftManagedDefault the static pods in charge of API and Ingress traffic load-balancing defined in the machine config operator will be deployed. When set to UserManaged these static pods will not be deployed and it is expected that the load balancer is configured out of band by the deployer. When omitted, this means no opinion and the platform is left to choose a reasonable default. The default value is OpenShiftManagedDefault. + enum: + - OpenShiftManagedDefault + - UserManaged + type: string + x-kubernetes-validations: + - message: type is immutable once set + rule: oldSelf == '' || self == oldSelf + type: object + nodeDNSIP: + description: nodeDNSIP is the IP address for the internal DNS used by the nodes. Unlike the one managed by the DNS operator, `NodeDNSIP` provides name resolution for the nodes themselves. There is no DNS-as-a-service for OpenStack deployments. In order to minimize necessary changes to the datacenter DNS, a DNS service is hosted as a static pod to serve those hostnames to the nodes in the cluster. + type: string + type: object + ovirt: + description: Ovirt contains settings specific to the oVirt infrastructure provider. + properties: + apiServerInternalIP: + description: "apiServerInternalIP is an IP address to contact the Kubernetes API server that can be used by components inside the cluster, like kubelets using the infrastructure rather than Kubernetes networking. It is the IP that the Infrastructure.status.apiServerInternalURI points to. It is the IP for a self-hosted load balancer in front of the API servers. \n Deprecated: Use APIServerInternalIPs instead." + type: string + apiServerInternalIPs: + description: apiServerInternalIPs are the IP addresses to contact the Kubernetes API server that can be used by components inside the cluster, like kubelets using the infrastructure rather than Kubernetes networking. These are the IPs for a self-hosted load balancer in front of the API servers. In dual stack clusters this list contains two IPs otherwise only one. + format: ip + items: + type: string + maxItems: 2 + type: array + ingressIP: + description: "ingressIP is an external IP which routes to the default ingress controller. The IP is a suitable target of a wildcard DNS record used to resolve default route host names. \n Deprecated: Use IngressIPs instead." + type: string + ingressIPs: + description: ingressIPs are the external IPs which route to the default ingress controller. The IPs are suitable targets of a wildcard DNS record used to resolve default route host names. In dual stack clusters this list contains two IPs otherwise only one. + format: ip + items: + type: string + maxItems: 2 + type: array + nodeDNSIP: + description: 'deprecated: as of 4.6, this field is no longer set or honored. It will be removed in a future release.' + type: string + type: object + powervs: + description: PowerVS contains settings specific to the Power Systems Virtual Servers infrastructure provider. + properties: + cisInstanceCRN: + description: CISInstanceCRN is the CRN of the Cloud Internet Services instance managing the DNS zone for the cluster's base domain + type: string + dnsInstanceCRN: + description: DNSInstanceCRN is the CRN of the DNS Services instance managing the DNS zone for the cluster's base domain + type: string + region: + description: region holds the default Power VS region for new Power VS resources created by the cluster. + type: string + resourceGroup: + description: 'resourceGroup is the resource group name for new IBMCloud resources created for a cluster. The resource group specified here will be used by cluster-image-registry-operator to set up a COS Instance in IBMCloud for the cluster registry. More about resource groups can be found here: https://cloud.ibm.com/docs/account?topic=account-rgs. When omitted, the image registry operator won''t be able to configure storage, which results in the image registry cluster operator not being in an available state.' + maxLength: 40 + pattern: ^[a-zA-Z0-9-_ ]+$ + type: string + x-kubernetes-validations: + - message: resourceGroup is immutable once set + rule: oldSelf == '' || self == oldSelf + serviceEndpoints: + description: serviceEndpoints is a list of custom endpoints which will override the default service endpoints of a Power VS service. + items: + description: PowervsServiceEndpoint stores the configuration of a custom url to override existing defaults of PowerVS Services. + properties: + name: + description: name is the name of the Power VS service. Few of the services are IAM - https://cloud.ibm.com/apidocs/iam-identity-token-api ResourceController - https://cloud.ibm.com/apidocs/resource-controller/resource-controller Power Cloud - https://cloud.ibm.com/apidocs/power-cloud + pattern: ^[a-z0-9-]+$ + type: string + url: + description: url is fully qualified URI with scheme https, that overrides the default generated endpoint for a client. This must be provided and cannot be empty. + format: uri + pattern: ^https:// + type: string + required: + - name + - url + type: object + type: array + zone: + description: 'zone holds the default zone for the new Power VS resources created by the cluster. Note: Currently only single-zone OCP clusters are supported' + type: string + type: object + x-kubernetes-validations: + - message: cannot unset resourceGroup once set + rule: '!has(oldSelf.resourceGroup) || has(self.resourceGroup)' + type: + description: "type is the underlying infrastructure provider for the cluster. This value controls whether infrastructure automation such as service load balancers, dynamic volume provisioning, machine creation and deletion, and other integrations are enabled. If None, no infrastructure automation is enabled. Allowed values are \"AWS\", \"Azure\", \"BareMetal\", \"GCP\", \"Libvirt\", \"OpenStack\", \"VSphere\", \"oVirt\", \"EquinixMetal\", \"PowerVS\", \"AlibabaCloud\", \"Nutanix\" and \"None\". Individual components may not support all platforms, and must handle unrecognized platforms as None if they do not support that platform. \n This value will be synced with to the `status.platform` and `status.platformStatus.type`. Currently this value cannot be changed once set." + enum: + - "" + - AWS + - Azure + - BareMetal + - GCP + - Libvirt + - OpenStack + - None + - VSphere + - oVirt + - IBMCloud + - KubeVirt + - EquinixMetal + - PowerVS + - AlibabaCloud + - Nutanix + - External + type: string + vsphere: + description: VSphere contains settings specific to the VSphere infrastructure provider. + properties: + apiServerInternalIP: + description: "apiServerInternalIP is an IP address to contact the Kubernetes API server that can be used by components inside the cluster, like kubelets using the infrastructure rather than Kubernetes networking. It is the IP that the Infrastructure.status.apiServerInternalURI points to. It is the IP for a self-hosted load balancer in front of the API servers. \n Deprecated: Use APIServerInternalIPs instead." + type: string + apiServerInternalIPs: + description: apiServerInternalIPs are the IP addresses to contact the Kubernetes API server that can be used by components inside the cluster, like kubelets using the infrastructure rather than Kubernetes networking. These are the IPs for a self-hosted load balancer in front of the API servers. In dual stack clusters this list contains two IPs otherwise only one. + format: ip + items: + type: string + maxItems: 2 + type: array + ingressIP: + description: "ingressIP is an external IP which routes to the default ingress controller. The IP is a suitable target of a wildcard DNS record used to resolve default route host names. \n Deprecated: Use IngressIPs instead." + type: string + ingressIPs: + description: ingressIPs are the external IPs which route to the default ingress controller. The IPs are suitable targets of a wildcard DNS record used to resolve default route host names. In dual stack clusters this list contains two IPs otherwise only one. + format: ip + items: + type: string + maxItems: 2 + type: array + nodeDNSIP: + description: nodeDNSIP is the IP address for the internal DNS used by the nodes. Unlike the one managed by the DNS operator, `NodeDNSIP` provides name resolution for the nodes themselves. There is no DNS-as-a-service for vSphere deployments. In order to minimize necessary changes to the datacenter DNS, a DNS service is hosted as a static pod to serve those hostnames to the nodes in the cluster. + type: string + type: object + type: object + type: object + required: + - spec + type: object + served: true + storage: true + subresources: + status: {} From 2712b01c7b2308fa4fd0b6dd8fba03a4bf8779c9 Mon Sep 17 00:00:00 2001 From: chiragkyal Date: Thu, 17 Oct 2024 17:35:12 +0530 Subject: [PATCH 2/3] add e2e test Signed-off-by: chiragkyal --- test/e2e/operator_test.go | 191 +++++++++++++++++++++++++++++++++++++- test/e2e/util.go | 51 ++++++++++ 2 files changed, 239 insertions(+), 3 deletions(-) diff --git a/test/e2e/operator_test.go b/test/e2e/operator_test.go index 55c81db2..05d7bf91 100644 --- a/test/e2e/operator_test.go +++ b/test/e2e/operator_test.go @@ -24,7 +24,6 @@ import ( "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/kubernetes" clientgoscheme "k8s.io/client-go/kubernetes/scheme" - kscheme "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/util/retry" "k8s.io/utils/pointer" @@ -59,13 +58,17 @@ const ( // controllerSecretName is the name of the controller's cloud credential secret provisioned by the CI. controllerSecretName = "aws-load-balancer-controller-cluster" + + // awsLoadBalancerControllerContainerName is the name of the AWS load balancer controller's container. + awsLoadBalancerControllerContainerName = "controller" ) var ( cfg aws.Config kubeClient client.Client kubeClientSet *kubernetes.Clientset - scheme = kscheme.Scheme + infra configv1.Infrastructure + scheme = clientgoscheme.Scheme operatorName = "aws-load-balancer-operator-controller-manager" operatorNamespace = "aws-load-balancer-operator" defaultTimeout = 15 * time.Minute @@ -113,7 +116,6 @@ func TestMain(m *testing.M) { os.Exit(1) } - var infra configv1.Infrastructure err = kubeClient.Get(context.TODO(), types.NamespacedName{Name: "cluster"}, &infra) if err != nil { fmt.Printf("failed to fetch infrastructure: %v\n", err) @@ -1199,6 +1201,140 @@ func TestAWSLoadBalancerControllerOpenAPIValidation(t *testing.T) { } } +// TestAWSLoadBalancerControllerUserTags verifies that the user-defined tags are correctly applied. +// It verifies that the controller deployment's `--default-tags` argument is updated +// to include the tags from both the controller spec and the infrastructure status, +// giving precedence to the controller spec in case of key conflicts. +func TestAWSLoadBalancerControllerUserTags(t *testing.T) { + managed, err := isManagedServiceCluster(context.TODO(), kubeClientSet) + if err != nil { + t.Fatalf("failed to check managed cluster %v", err) + } + if managed { + t.Skip("Infrastructure status cannot be directly updated on managed cluster, skipping...") + } + + testWorkloadNamespace := "aws-load-balancer-test-user-tags" + t.Logf("Creating test namespace %q", testWorkloadNamespace) + echoNs := createTestNamespace(t, testWorkloadNamespace) + defer func() { + waitForDeletion(context.TODO(), t, kubeClient, echoNs, defaultTimeout) + }() + + t.Log("Creating aws load balancer controller instance with default ingress class and user tags") + + alb := newALBCBuilder().withRoleARNIf(stsModeRequested(), controllerRoleARN).build() + // add additional resource tags in alb spec + alb.Spec.AdditionalResourceTags = []albo.AWSResourceTag{ + {Key: "op-key1", Value: "op-value1"}, + {Key: "conflict-key1", Value: "op-value2"}, + {Key: "conflict-key2", Value: "op-value3"}, + } + + if err := kubeClient.Create(context.TODO(), alb); err != nil { + t.Fatalf("failed to create aws load balancer controller: %v", err) + } + defer func() { + waitForDeletion(context.TODO(), t, kubeClient, alb, defaultTimeout) + }() + + expected := []appsv1.DeploymentCondition{ + {Type: appsv1.DeploymentAvailable, Status: corev1.ConditionTrue}, + } + deploymentName := types.NamespacedName{Name: "aws-load-balancer-controller-cluster", Namespace: "aws-load-balancer-operator"} + if err := waitForDeploymentStatusCondition(context.TODO(), t, kubeClient, defaultTimeout, deploymentName, expected...); err != nil { + t.Fatalf("did not get expected available condition for deployment: %v", err) + } + + dep := &appsv1.Deployment{} + if err := kubeClient.Get(context.TODO(), deploymentName, dep); err != nil { + t.Fatalf("failed to get deployment %s: %v", deploymentName.Name, err) + } + depGeneration := dep.Generation + + // Save a copy of the original infra Config, to revert changes before exiting. + originalInfra := infra.DeepCopy() + defer func() { + err := updateInfrastructureConfigStatusWithRetryOnConflict(t, 5*time.Minute, kubeClient, func(infra *configv1.Infrastructure) *configv1.Infrastructure { + infra.Status = originalInfra.Status + return infra + }) + if err != nil { + t.Logf("Unable to remove changes from the infraConfig, possible corruption of test environment: %v", err) + } + }() + + // Update infrastructure status with initialInfraTags + initialInfraTags := []configv1.AWSResourceTag{ + {Key: "plat-key1", Value: "plat-value1"}, + {Key: "conflict-key1", Value: "plat-value2"}, + {Key: "conflict-key2", Value: "plat-value3"}, + } + t.Logf("Updating AWS ResourceTags in the cluster infrastructure config: %v", initialInfraTags) + err = updateInfrastructureConfigStatusWithRetryOnConflict(t, 5*time.Minute, kubeClient, func(infra *configv1.Infrastructure) *configv1.Infrastructure { + if infra.Status.PlatformStatus == nil { + infra.Status.PlatformStatus = &configv1.PlatformStatus{} + } + if infra.Status.PlatformStatus.AWS == nil { + infra.Status.PlatformStatus.AWS = &configv1.AWSPlatformStatus{} + } + infra.Status.PlatformStatus.AWS.ResourceTags = initialInfraTags + return infra + }) + if err != nil { + t.Errorf("failed to update infrastructure status: %v", err) + } + + // wait for the deployment to restart and become available + if err := waitForDeploymentRollout(context.TODO(), t, kubeClient, defaultTimeout, deploymentName, depGeneration); err != nil { + t.Fatalf("deployment did not roll out within timeout: %v", err) + } + if err := waitForDeploymentStatusCondition(context.TODO(), t, kubeClient, defaultTimeout, deploymentName, expected...); err != nil { + t.Fatalf("did not get expected available condition for deployment: %v", err) + } + + // get the updated deployment + if err := kubeClient.Get(context.TODO(), deploymentName, dep); err != nil { + t.Fatalf("failed to get deployment %s: %v", deploymentName.Name, err) + } + depGeneration = dep.Generation + + // Check `--default-tags` arg for tags present in alb instance and infra status (initialInfraTags) + expectedTagValue := "conflict-key1=op-value2,conflict-key2=op-value3,op-key1=op-value1,plat-key1=plat-value1" + assertContainerArgFromDeployment(t, dep, awsLoadBalancerControllerContainerName, "--default-tags", expectedTagValue) + + // Update the status again, removing one tag. + updatedInfraTags := []configv1.AWSResourceTag{ + {Key: "conflict-key1", Value: "plat-value2"}, + {Key: "conflict-key2", Value: "plat-value3"}, + } + t.Logf("Updating AWS ResourceTags in the cluster infrastructure config: %v", updatedInfraTags) + err = updateInfrastructureConfigStatusWithRetryOnConflict(t, 5*time.Minute, kubeClient, func(infra *configv1.Infrastructure) *configv1.Infrastructure { + infra.Status.PlatformStatus.AWS.ResourceTags = updatedInfraTags + return infra + }) + if err != nil { + t.Errorf("failed to update infrastructure status: %v", err) + } + + // wait for the deployment to restart and become available + if err := waitForDeploymentRollout(context.TODO(), t, kubeClient, defaultTimeout, deploymentName, depGeneration); err != nil { + t.Fatalf("deployment did not roll out within timeout: %v", err) + } + if err := waitForDeploymentStatusCondition(context.TODO(), t, kubeClient, defaultTimeout, deploymentName, expected...); err != nil { + t.Fatalf("did not get expected available condition for deployment: %v", err) + } + + // get the updated deployment + if err := kubeClient.Get(context.TODO(), deploymentName, dep); err != nil { + t.Fatalf("failed to get deployment %s: %v", deploymentName.Name, err) + } + + // Check `--default-tags` arg for tags present in alb instance and infra status (updatedInfraTags) + expectedTagValue = "conflict-key1=op-value2,conflict-key2=op-value3,op-key1=op-value1" + assertContainerArgFromDeployment(t, dep, awsLoadBalancerControllerContainerName, "--default-tags", expectedTagValue) +} + // ensureCredentialsRequest creates CredentialsRequest to provision a secret with the cloud credentials required by this e2e test. func ensureCredentialsRequest(secret types.NamespacedName) error { providerSpec, err := cco.Codec.EncodeProviderSpec(&cco.AWSProviderSpec{ @@ -1284,3 +1420,52 @@ func stsModeRequested() bool { } return false } + +// extractArg extracts the value of a specific argument from a list of container arguments. +// Returns an error if the argument is present but malformed, or if not found. +func extractArg(args []string, argKey string) (string, error) { + for _, arg := range args { + if strings.HasPrefix(arg, argKey+"=") { + parts := strings.SplitN(arg, "=", 2) + if len(parts) != 2 { + return "", fmt.Errorf("invalid format for argument %s: %s", argKey, arg) + } + return parts[1], nil + } + } + return "", fmt.Errorf("argument %s not found", argKey) +} + +// assertContainerArgFromDeployment asserts that a container in a deployment has a +// specific argument with an expected value. +func assertContainerArgFromDeployment(t *testing.T, dep *appsv1.Deployment, containerName, argKey, expectedArgValue string) { + t.Helper() + t.Logf("Asserting container %q has argument %q with value %q", containerName, argKey, expectedArgValue) + for _, c := range dep.Spec.Template.Spec.Containers { + if c.Name == containerName { + actualArgValue, err := extractArg(c.Args, argKey) + if err != nil { + t.Fatalf("failed to extract argument %q for container %q: %v", argKey, containerName, err) + } + + if actualArgValue != expectedArgValue { + t.Fatalf("container %q, argument %q: expected value %q, but got %q", containerName, argKey, expectedArgValue, actualArgValue) + } + return + } + } + t.Fatalf("container %q not found in deployment", containerName) +} + +func isManagedServiceCluster(ctx context.Context, adminClient kubernetes.Interface) (bool, error) { + _, err := adminClient.CoreV1().Namespaces().Get(ctx, "openshift-backplane", v1.GetOptions{}) + if err == nil { + return true, nil + } + + if !errors.IsNotFound(err) { + return false, err + } + + return false, nil +} diff --git a/test/e2e/util.go b/test/e2e/util.go index 806d2773..fd745404 100644 --- a/test/e2e/util.go +++ b/test/e2e/util.go @@ -41,6 +41,28 @@ var ( allowPrivilegeEscalation = false ) +// waitForDeploymentRollout waits for a deployment to complete a rollout restart +// by observing its generation. +func waitForDeploymentRollout(ctx context.Context, t *testing.T, cl client.Client, timeout time.Duration, deploymentName types.NamespacedName, oldGeneration int64) error { + t.Helper() + + return wait.PollUntilContextTimeout(ctx, time.Second, timeout, true, func(ctx context.Context) (bool, error) { + updatedDep := &appsv1.Deployment{} + if err := cl.Get(ctx, deploymentName, updatedDep); err != nil { + t.Logf("failed to get deployment %s: %v (retrying)", deploymentName.Name, err) + return false, err + } + + // Check if the deployment has progressed to the next generation and has atleast one ready replica + if updatedDep.Status.ObservedGeneration > oldGeneration && updatedDep.Status.ReadyReplicas > 0 { + return true, nil + } + + t.Logf("retrying for deployment rollout: observedGeneration: %d, readyReplicas: %d", updatedDep.Status.ObservedGeneration, updatedDep.Status.ReadyReplicas) + return false, nil + }) +} + func waitForDeploymentStatusCondition(ctx context.Context, t *testing.T, cl client.Client, timeout time.Duration, deploymentName types.NamespacedName, conditions ...appsv1.DeploymentCondition) error { t.Helper() @@ -509,3 +531,32 @@ func mustGetEnv(name string) string { } return val } + +// updateInfrastructureStatus updates the Infrastructure status by applying +// the given update function to the current Infrastructure object. +// If there is a conflict error on update then the complete operation +// is retried until timeout is reached. +func updateInfrastructureConfigStatusWithRetryOnConflict(t *testing.T, timeout time.Duration, kubeClient client.Client, updateFunc func(*configv1.Infrastructure) *configv1.Infrastructure) error { + t.Helper() + + t.Log("Updating 'cluster' infrastructure config status") + return wait.PollUntilContextTimeout(context.Background(), 5*time.Second, timeout, true, func(ctx context.Context) (bool, error) { + infra := &configv1.Infrastructure{} + if err := kubeClient.Get(context.TODO(), types.NamespacedName{Name: "cluster"}, infra); err != nil { + t.Logf("error getting 'cluster' infrastructure config: %v, retrying...", err) + return false, nil + } + + // Apply the update function to the Infrastructure object. + updatedInfra := updateFunc(infra.DeepCopy()) + + if err := kubeClient.Status().Update(context.TODO(), updatedInfra); err != nil { + if errors.IsConflict(err) { + t.Logf("conflict when updating 'cluster' infrastructure config: %v, retrying...", err) + return false, nil + } + return false, err + } + return true, nil + }) +} From e9077da2d025d42bfc9a3fc1fa3041d5d7fca7b0 Mon Sep 17 00:00:00 2001 From: chiragkyal Date: Fri, 15 Nov 2024 16:39:43 +0530 Subject: [PATCH 3/3] Address review comments Signed-off-by: chiragkyal --- .../awsloadbalancercontroller/deployment.go | 10 ++++------ .../deployment_test.go | 6 +++--- test/e2e/operator_test.go | 19 ++++++++++++------- 3 files changed, 19 insertions(+), 16 deletions(-) diff --git a/pkg/controllers/awsloadbalancercontroller/deployment.go b/pkg/controllers/awsloadbalancercontroller/deployment.go index c12db7d2..3acf3942 100644 --- a/pkg/controllers/awsloadbalancercontroller/deployment.go +++ b/pkg/controllers/awsloadbalancercontroller/deployment.go @@ -127,7 +127,7 @@ func (r *AWSLoadBalancerControllerReconciler) ensureDeployment(ctx context.Conte } func (r *AWSLoadBalancerControllerReconciler) desiredDeployment(name, credentialsRequestSecretName, servingSecret string, controller *albo.AWSLoadBalancerController, platformStatus *configv1.PlatformStatus, sa *corev1.ServiceAccount, trustedCAConfigMapName, trustedCAConfigMapHash string) (*appsv1.Deployment, error) { - containerArgs, err := desiredContainerArgs(controller, platformStatus, r.ClusterName, r.VPCID) + containerArgs, err := desiredContainerArgs(controller, r.ClusterName, r.VPCID, platformStatus) if err != nil { return nil, fmt.Errorf("failed to get container args: %w", err) } @@ -270,7 +270,7 @@ func (r *AWSLoadBalancerControllerReconciler) desiredDeployment(name, credential return d, nil } -func desiredContainerArgs(controller *albo.AWSLoadBalancerController, platformStatus *configv1.PlatformStatus, clusterName, vpcID string) ([]string, error) { +func desiredContainerArgs(controller *albo.AWSLoadBalancerController, clusterName, vpcID string, platformStatus *configv1.PlatformStatus) ([]string, error) { var args []string args = append(args, fmt.Sprintf("--webhook-cert-dir=%s", webhookTLSDir)) args = append(args, fmt.Sprintf("--aws-vpc-id=%s", vpcID)) @@ -572,10 +572,8 @@ func mergeTags(controller *albo.AWSLoadBalancerController, platformStatus *confi // Add tags from platformStatus.AWS.ResourceTags to the map, only if the key doesn't exist if platformStatus.AWS != nil && len(platformStatus.AWS.ResourceTags) > 0 { for _, t := range platformStatus.AWS.ResourceTags { - if len(t.Key) > 0 { - if _, exists := tagMap[t.Key]; !exists { - tagMap[t.Key] = t.Value - } + if _, exists := tagMap[t.Key]; !exists { + tagMap[t.Key] = t.Value } } } diff --git a/pkg/controllers/awsloadbalancercontroller/deployment_test.go b/pkg/controllers/awsloadbalancercontroller/deployment_test.go index e65d2b8c..c2b16095 100644 --- a/pkg/controllers/awsloadbalancercontroller/deployment_test.go +++ b/pkg/controllers/awsloadbalancercontroller/deployment_test.go @@ -242,7 +242,7 @@ func TestDesiredArgs(t *testing.T) { if tc.platformStatus == nil { tc.platformStatus = &configv1.PlatformStatus{} } - args, gotErr := desiredContainerArgs(tc.controller, tc.platformStatus, "test-cluster", "test-vpc") + args, gotErr := desiredContainerArgs(tc.controller, "test-cluster", "test-vpc", tc.platformStatus) if (gotErr != nil) != tc.expectedError { t.Fatalf("expected errors to be %t, but got %t", tc.expectedError, gotErr != nil) } @@ -851,7 +851,7 @@ func TestEnsureDeployment(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - args, err := desiredContainerArgs(tc.controller, &configv1.PlatformStatus{}, "test-cluster", "test-vpc") + args, err := desiredContainerArgs(tc.controller, "test-cluster", "test-vpc", &configv1.PlatformStatus{}) if err != nil { t.Fatalf("failed to get container args: %v", err) } @@ -966,7 +966,7 @@ func TestEnsureDeploymentEnvVars(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - args, err := desiredContainerArgs(tc.controller, &configv1.PlatformStatus{}, "test-cluster", "test-vpc") + args, err := desiredContainerArgs(tc.controller, "test-cluster", "test-vpc", &configv1.PlatformStatus{}) if err != nil { t.Fatalf("failed to get container args: %v", err) } diff --git a/test/e2e/operator_test.go b/test/e2e/operator_test.go index 05d7bf91..1b335057 100644 --- a/test/e2e/operator_test.go +++ b/test/e2e/operator_test.go @@ -1223,13 +1223,15 @@ func TestAWSLoadBalancerControllerUserTags(t *testing.T) { t.Log("Creating aws load balancer controller instance with default ingress class and user tags") - alb := newALBCBuilder().withRoleARNIf(stsModeRequested(), controllerRoleARN).build() - // add additional resource tags in alb spec - alb.Spec.AdditionalResourceTags = []albo.AWSResourceTag{ - {Key: "op-key1", Value: "op-value1"}, - {Key: "conflict-key1", Value: "op-value2"}, - {Key: "conflict-key2", Value: "op-value3"}, - } + // create alb with additional resource tags added in alb spec + alb := newALBCBuilder(). + withRoleARNIf(stsModeRequested(), controllerRoleARN). + withResourceTags(map[string]string{ + "op-key1": "op-value1", + "conflict-key1": "op-value2", + "conflict-key2": "op-value3", + }). + build() if err := kubeClient.Create(context.TODO(), alb); err != nil { t.Fatalf("failed to create aws load balancer controller: %v", err) @@ -1457,6 +1459,9 @@ func assertContainerArgFromDeployment(t *testing.T, dep *appsv1.Deployment, cont t.Fatalf("container %q not found in deployment", containerName) } +// This logic was inspired by +// https://github.com/openshift/origin/pull/29216/files#diff-35a89a7a7362642eebb559fb8564e857b00d6f7dd6322c3adabaf1adbd609d35R2267-R2278 +// implementation. func isManagedServiceCluster(ctx context.Context, adminClient kubernetes.Interface) (bool, error) { _, err := adminClient.CoreV1().Namespaces().Get(ctx, "openshift-backplane", v1.GetOptions{}) if err == nil {