From e7fd7e533829cc0d9926f9487df3a9a7d792ae87 Mon Sep 17 00:00:00 2001 From: Bharath B Date: Thu, 8 Aug 2024 13:45:56 +0530 Subject: [PATCH] CFE-877: Add resourceTags in Infrastructure to driver args list --- assets/credentials.yaml | 6 +- pkg/operator/starter.go | 42 ++++++++++- pkg/operator/starter_test.go | 139 +++++++++++++++++++++++++++++++++++ 3 files changed, 185 insertions(+), 2 deletions(-) diff --git a/assets/credentials.yaml b/assets/credentials.yaml index 991d9ab1c..c0748625c 100644 --- a/assets/credentials.yaml +++ b/assets/credentials.yaml @@ -18,6 +18,10 @@ spec: kind: GCPProviderSpec predefinedRoles: - roles/file.editor + - roles/resourcemanager.tagUser # If set to true, don't check whether the requested # roles have the necessary services enabled - skipServiceCheck: false + # roles/resourcemanager.tagUser requires certain services to be activated + # which are not necessarily required for creating OpenShift cluster required + # resources. Hence skipping service check. + skipServiceCheck: true diff --git a/pkg/operator/starter.go b/pkg/operator/starter.go index 672fc2d58..98c70de3f 100644 --- a/pkg/operator/starter.go +++ b/pkg/operator/starter.go @@ -129,6 +129,7 @@ func RunOperator(ctx context.Context, controllerConfig *controllercmd.Controller ), csidrivercontrollerservicecontroller.WithReplicasHook(nodeInformer.Lister()), withCustomLabels(infraInformer.Lister()), + withCustomResourceTags(infraInformer.Lister()), ).WithCSIDriverNodeService( "GCPFilestoreDriverNodeServiceController", replaceNamespaceFunc(operatorNamespace), @@ -246,7 +247,7 @@ func withCustomLabels(infraLister configlisters.InfrastructureLister) dc.Deploym labels = append(labels, fmt.Sprintf(ocpDefaultLabelFmt, infra.Status.InfrastructureName)) labelsStr := strings.Join(labels, ",") labelsArg := fmt.Sprintf("--extra-labels=%s", labelsStr) - klog.V(1).Infof("withCustomLabels: adding extra-labels arg to driver with value %s", labelsStr) + klog.V(5).Infof("withCustomLabels: adding extra-labels arg to driver with value %s", labelsStr) for i := range deployment.Spec.Template.Spec.Containers { container := &deployment.Spec.Template.Spec.Containers[i] @@ -258,3 +259,42 @@ func withCustomLabels(infraLister configlisters.InfrastructureLister) dc.Deploym return nil } } + +// withCustomResourceTags adds resource tags from infrastructure.status.platformStatus.gcp.resourceTags to the +// driver command line as --resource-tags=//,... +func withCustomResourceTags(infraLister configlisters.InfrastructureLister) dc.DeploymentHookFunc { + return func(spec *opv1.OperatorSpec, deployment *appsv1.Deployment) error { + infra, err := infraLister.Get(globalInfrastructureName) + if err != nil { + return fmt.Errorf("withCustomResourceTags: failed to fetch global Infrastructure object: %w", err) + } + + var tags []string + if infra.Status.PlatformStatus != nil && + infra.Status.PlatformStatus.GCP != nil && + infra.Status.PlatformStatus.GCP.ResourceTags != nil { + tags = make([]string, len(infra.Status.PlatformStatus.GCP.ResourceTags)) + for i, tag := range infra.Status.PlatformStatus.GCP.ResourceTags { + tags[i] = fmt.Sprintf("%s/%s/%s", tag.ParentID, tag.Key, tag.Value) + } + } + + if len(tags) <= 0 { + klog.V(5).Infof("withCustomResourceTags: user tags not configured, no changes made to driver args") + return nil + } + + tagsStr := strings.Join(tags, ",") + tagsArg := fmt.Sprintf("--resource-tags=%s", tagsStr) + klog.V(5).Infof("withCustomResourceTags: adding resource-tags arg to driver with value %s", tagsStr) + + for i := range deployment.Spec.Template.Spec.Containers { + container := &deployment.Spec.Template.Spec.Containers[i] + if container.Name != "csi-driver" { + continue + } + container.Args = append(container.Args, tagsArg) + } + return nil + } +} diff --git a/pkg/operator/starter_test.go b/pkg/operator/starter_test.go index 293cd8916..7ba8287fa 100644 --- a/pkg/operator/starter_test.go +++ b/pkg/operator/starter_test.go @@ -152,3 +152,142 @@ func TestWithCustomLabels(t *testing.T) { }) } } + +func TestWithCustomResourceTags(t *testing.T) { + infraObj := &v1.Infrastructure{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster", + }, + Status: v1.InfrastructureStatus{ + InfrastructureName: "test-dfgh2", + PlatformStatus: &v1.PlatformStatus{ + GCP: &v1.GCPPlatformStatus{ + ProjectID: "test", + Region: "test", + }, + }, + }, + } + + tmplDeployObj := &appsv1.Deployment{ + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "csi-driver", + Image: "example.io/example-csi-driver", + Args: []string{ + "--endpoint=$(CSI_ENDPOINT)", + "--logtostderr", + "--v=2", + "--nodeid=2", + "--controller=true", + }, + Env: []corev1.EnvVar{ + { + Name: "GOOGLE_APPLICATION_CREDENTIALS", + Value: "/etc/cloud-sa/service_account.json", + }, + { + Name: "CSI_ENDPOINT", + Value: "unix:///var/lib/csi/sockets/pluginproxy/csi.sock", + }, + { + Name: "KUBE_NODE_NAME", + ValueFrom: &corev1.EnvVarSource{ + FieldRef: &corev1.ObjectFieldSelector{ + FieldPath: "test", + }, + }, + }, + }, + }, + { + Name: "test-driver", + Image: "example.io/example-test-driver", + }, + }, + }, + }, + }, + } + + tests := []struct { + name string + tags []v1.GCPResourceTag + expArgList string + createInfraCR bool + wantErr bool + }{ + { + name: "user tags not configured", + tags: []v1.GCPResourceTag{}, + expArgList: "", + createInfraCR: true, + wantErr: false, + }, + { + name: "user tags configured", + tags: []v1.GCPResourceTag{ + { + ParentID: "openshift", + Key: "key1", + Value: "value1", + }, + { + ParentID: "openshift", + Key: "key2", + Value: "value2", + }, + { + ParentID: "openshift", + Key: "key3", + Value: "value3", + }, + }, + expArgList: "--resource-tags=openshift/key1/value1,openshift/key2/value2,openshift/key3/value3", + createInfraCR: true, + wantErr: false, + }, + { + name: "Infrastructure CR does not exist", + tags: []v1.GCPResourceTag{}, + expArgList: "", + createInfraCR: false, + wantErr: true, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + objs := make([]runtime.Object, 0) + if test.createInfraCR { + infraObj.Status.PlatformStatus.GCP.ResourceTags = test.tags + objs = append(objs, infraObj) + } + configClient := fakeconfig.NewSimpleClientset(objs...) + configInformerFactory := configinformers.NewSharedInformerFactory(configClient, 0) + if test.createInfraCR { + configInformerFactory.Config().V1().Infrastructures().Informer().GetIndexer().Add(infraObj) + } + + deployment := tmplDeployObj.DeepCopy() + updDeployment := tmplDeployObj.DeepCopy() + if test.expArgList != "" { + updDeployment.Spec.Template.Spec.Containers[0].Args = append( + updDeployment.Spec.Template.Spec.Containers[0].Args, + test.expArgList, + ) + } + + err := withCustomResourceTags(configInformerFactory.Config().V1().Infrastructures().Lister())(nil, deployment) + if (err != nil) != test.wantErr { + t.Errorf("unexpected error: %v", err) + } + if !equality.Semantic.DeepEqual(deployment, updDeployment) { + t.Errorf("unexpected deployment want: %+v got: %+v", updDeployment, deployment) + } + }) + } +}