Skip to content

CFE-877: Add resourceTags in Infrastructure to driver args list #92

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
openshift-merge-bot merged 1 commit into from
Aug 8, 2024
Merged

CFE-877: Add resourceTags in Infrastructure to driver args list #92

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion assets/credentials.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Copy Markdown
Contributor Author

@bharath-b-rh bharath-b-rh Aug 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

skipServiceCheck is enabled, since roles/resourcemanager.tagUser requires certain services to be enabled, which are not necessarily required for creating OpenShift cluster required resources.

time="2024-08-08T08:06:07Z" level=warning msg="Detected required APIs that are disabled: [alloydb.googleapis.com clouddeploy.googleapis.com cloudsql.googleapis.com datafusion.googleapis.com datastream.googleapis.com domains.googleapis.com managedidentities.googleapis.com]" actuator=gcp cr=openshift-cloud-credential-operator/openshift-gcp-filestore-csi-driver-operator
time="2024-08-08T08:06:07Z" level=error msg="not all required service APIs are enabled" actuator=gcp cr=openshift-cloud-credential-operator/openshift-gcp-filestore-csi-driver-operator
time="2024-08-08T08:06:07Z" level=error msg="error syncing credentials: not all required service APIs are enabled" controller=credreq cr=openshift-cloud-credential-operator/openshift-gcp-filestore-csi-driver-operator secret=openshift-cluster-csi-drivers/gcp-filestore-cloud-credentials
time="2024-08-08T08:06:07Z" level=error msg="errored with condition: CredentialsProvisionFailure" controller=credreq cr=openshift-cloud-credential-operator/openshift-gcp-filestore-csi-driver-operator secret=openshift-cluster-csi-drivers/gcp-filestore-cloud-credentials

42 changes: 41 additions & 1 deletion pkg/operator/starter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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]
Expand All @@ -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=<parent_id>/<tagKey_shortname>/<tagValue_shortname>,...
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
}
}
139 changes: 139 additions & 0 deletions pkg/operator/starter_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
}
}