Skip to content

Commit 8b509a4

Browse files
refactor: setting ownership references to Nutanix CSI Helm Chart Proxies (#565)
**What problem does this PR solve?**: I noticed what appeared to be a missing ownership reference. @faiq confirmed it as an oversight. This change also refactors some of the surrounding code. Many handlers have very similar code for CAAPH-based installs. In a future PR, we can factor this code into its own package, where we can enforce rules like setting ownership references, and avoid this kind of oversight. **Which issue(s) this PR fixes**: Fixes # **How Has This Been Tested?**: <!-- Please describe the tests that you ran to verify your changes. Provide output from the tests and any manual steps needed to replicate the tests. --> **Special notes for your reviewer**: <!-- Use this to provide any additional information to the reviewers. This may include: - Best way to review the PR. - Where the author wants the most review attention on. - etc. --> --------- Co-authored-by: Jimmi Dyson <[email protected]>
1 parent 3ad5620 commit 8b509a4

File tree

1 file changed

+27
-37
lines changed
  • pkg/handlers/generic/lifecycle/csi/nutanix-csi

1 file changed

+27
-37
lines changed

pkg/handlers/generic/lifecycle/csi/nutanix-csi/handler.go

+27-37
Original file line numberDiff line numberDiff line change
@@ -151,12 +151,17 @@ func (n *NutanixCSI) handleHelmAddonApply(
151151
)
152152
}
153153

154-
helmChart, err := n.helmChartInfoGetter.For(ctx, log, config.NutanixStorageCSI)
154+
storageChart, err := n.helmChartInfoGetter.For(ctx, log, config.NutanixStorageCSI)
155155
if err != nil {
156-
return fmt.Errorf("failed to get values for nutanix-csi-config %w", err)
156+
return fmt.Errorf("failed to get helm chart %q: %w", config.NutanixStorageCSI, err)
157157
}
158158

159-
hcp := &caaphv1.HelmChartProxy{
159+
snapshotChart, err := n.helmChartInfoGetter.For(ctx, log, config.NutanixSnapshotCSI)
160+
if err != nil {
161+
return fmt.Errorf("failed to get helm chart %q: %w", config.NutanixSnapshotCSI, err)
162+
}
163+
164+
storageChartProxy := &caaphv1.HelmChartProxy{
160165
TypeMeta: metav1.TypeMeta{
161166
APIVersion: caaphv1.GroupVersion.String(),
162167
Kind: "HelmChartProxy",
@@ -166,35 +171,19 @@ func (n *NutanixCSI) handleHelmAddonApply(
166171
Name: "nutanix-csi-" + req.Cluster.Name,
167172
},
168173
Spec: caaphv1.HelmChartProxySpec{
169-
RepoURL: helmChart.Repository,
170-
ChartName: helmChart.Name,
174+
RepoURL: storageChart.Repository,
175+
ChartName: storageChart.Name,
171176
ClusterSelector: metav1.LabelSelector{
172177
MatchLabels: map[string]string{clusterv1.ClusterNameLabel: req.Cluster.Name},
173178
},
174179
ReleaseNamespace: defaultStorageHelmReleaseNamespace,
175180
ReleaseName: defaultStorageHelmReleaseName,
176-
Version: helmChart.Version,
181+
Version: storageChart.Version,
177182
ValuesTemplate: values,
178183
},
179184
}
180185

181-
if err = controllerutil.SetOwnerReference(&req.Cluster, hcp, n.client.Scheme()); err != nil {
182-
return fmt.Errorf(
183-
"failed to set owner reference on nutanix-csi installation HelmChartProxy: %w",
184-
err,
185-
)
186-
}
187-
188-
if err = client.ServerSideApply(ctx, n.client, hcp, client.ForceOwnership); err != nil {
189-
return fmt.Errorf("failed to apply nutanix-csi installation HelmChartProxy: %w", err)
190-
}
191-
192-
snapshotHelmChart, err := n.helmChartInfoGetter.For(ctx, log, config.NutanixSnapshotCSI)
193-
if err != nil {
194-
return fmt.Errorf("failed to get values for nutanix-csi-config %w", err)
195-
}
196-
197-
snapshotChart := &caaphv1.HelmChartProxy{
186+
snapshotChartProxy := &caaphv1.HelmChartProxy{
198187
TypeMeta: metav1.TypeMeta{
199188
APIVersion: caaphv1.GroupVersion.String(),
200189
Kind: "HelmChartProxy",
@@ -204,29 +193,30 @@ func (n *NutanixCSI) handleHelmAddonApply(
204193
Name: "nutanix-csi-snapshot-" + req.Cluster.Name,
205194
},
206195
Spec: caaphv1.HelmChartProxySpec{
207-
RepoURL: snapshotHelmChart.Repository,
208-
ChartName: snapshotHelmChart.Name,
196+
RepoURL: snapshotChart.Repository,
197+
ChartName: snapshotChart.Name,
209198
ClusterSelector: metav1.LabelSelector{
210199
MatchLabels: map[string]string{clusterv1.ClusterNameLabel: req.Cluster.Name},
211200
},
212201
ReleaseNamespace: defaultSnapshotHelmReleaseNamespace,
213202
ReleaseName: defaultSnapshotHelmReleaseName,
214-
Version: snapshotHelmChart.Version,
203+
Version: snapshotChart.Version,
215204
},
216205
}
217206

218-
if err = controllerutil.SetOwnerReference(&req.Cluster, snapshotChart, n.client.Scheme()); err != nil {
219-
return fmt.Errorf(
220-
"failed to set owner reference on nutanix-csi-snapshot installation HelmChartProxy: %w",
221-
err,
222-
)
223-
}
207+
// We use a slice of pointers to satisfy the gocritic linter rangeValCopy check.
208+
for _, cp := range []*caaphv1.HelmChartProxy{storageChartProxy, snapshotChartProxy} {
209+
if err = controllerutil.SetOwnerReference(&req.Cluster, cp, n.client.Scheme()); err != nil {
210+
return fmt.Errorf(
211+
"failed to set owner reference on HelmChartProxy %q: %w",
212+
cp.Name,
213+
err,
214+
)
215+
}
224216

225-
if err = client.ServerSideApply(ctx, n.client, snapshotChart); err != nil {
226-
return fmt.Errorf(
227-
"failed to apply nutanix-csi-snapshot installation HelmChartProxy: %w",
228-
err,
229-
)
217+
if err = client.ServerSideApply(ctx, n.client, cp, client.ForceOwnership); err != nil {
218+
return fmt.Errorf("failed to apply HelmChartProxy %q: %w", cp.Name, err)
219+
}
230220
}
231221

232222
return nil

0 commit comments

Comments
 (0)