Skip to content

Commit

Permalink
feat: Preserve user-managed fields when applying resources (#556)
Browse files Browse the repository at this point in the history
**What problem does this PR solve?**:
We want to preserve user-managed fields when we create/update specific
resources. If a value in a user-managed field conflicts with our value,
we want to alert the user by returning an error, rather than overwriting
the value.

This PR exposes the `ForceOwnership` option to callers, but does not
change the behavior of existing callers.

**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.
-->
  • Loading branch information
dlipovetsky committed Apr 25, 2024
1 parent db51743 commit 36a0662
Show file tree
Hide file tree
Showing 17 changed files with 38 additions and 34 deletions.
34 changes: 19 additions & 15 deletions common/pkg/k8s/client/apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,25 +10,29 @@ import (
ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
)

// ServerSideApply will apply (i.e. create or update) objects via server-side apply. This will overwrite any changes
// that have been manually applied.
const (
// FieldOwner is the field manager name used for server-side apply.
FieldOwner = "d2iq-cluster-api-runtime-extensions-nutanix"
)

// ForceOwnership is an convenience alias of the same option in the controller-runtime client.
var ForceOwnership = ctrlclient.ForceOwnership

// ServerSideApply will apply (i.e. create or update) objects via server-side apply.
func ServerSideApply(
ctx context.Context,
c ctrlclient.Client,
objs ...ctrlclient.Object,
obj ctrlclient.Object,
opts ...ctrlclient.PatchOption,
) error {
for i := range objs {
err := c.Patch(
ctx,
objs[i],
ctrlclient.Apply,
ctrlclient.ForceOwnership,
ctrlclient.FieldOwner("d2iq-cluster-api-runtime-extensions-nutanix"),
)
if err != nil {
return fmt.Errorf("server-side apply failed: %w", err)
}
err := c.Patch(
ctx,
obj,
ctrlclient.Apply,
ctrlclient.FieldOwner(FieldOwner),
)
if err != nil {
return fmt.Errorf("server-side apply failed: %w", err)
}

return nil
}
2 changes: 1 addition & 1 deletion pkg/handlers/generic/lifecycle/ccm/aws/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func (a *AWSCCM) Apply(
}

ccmConfigMap := generateCCMConfigMapForCluster(ccmConfigMapForMinorVersion, cluster)
if err = client.ServerSideApply(ctx, a.client, ccmConfigMap); err != nil {
if err = client.ServerSideApply(ctx, a.client, ccmConfigMap, client.ForceOwnership); err != nil {
log.Error(err, "failed to apply CCM configmap for cluster")
return fmt.Errorf(
"failed to apply AWS CCM manifests ConfigMap: %w",
Expand Down
2 changes: 1 addition & 1 deletion pkg/handlers/generic/lifecycle/ccm/nutanix/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ func (p *provider) Apply(
)
}

if err = client.ServerSideApply(ctx, p.client, hcp); err != nil {
if err = client.ServerSideApply(ctx, p.client, hcp, client.ForceOwnership); err != nil {
return fmt.Errorf("failed to apply nutanix-ccm installation HelmChartProxy: %w", err)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ func (s crsStrategy) apply(
Data: data,
}

if err := client.ServerSideApply(ctx, s.client, cm); err != nil {
if err := client.ServerSideApply(ctx, s.client, cm, client.ForceOwnership); err != nil {
return fmt.Errorf(
"failed to apply cluster-autoscaler installation ConfigMap: %w",
err,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ func (s helmAddonStrategy) apply(
)
}

if err = client.ServerSideApply(ctx, s.client, hcp); err != nil {
if err = client.ServerSideApply(ctx, s.client, hcp, client.ForceOwnership); err != nil {
return fmt.Errorf("failed to apply cluster-autoscaler installation HelmChartProxy: %w", err)
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/handlers/generic/lifecycle/cni/calico/strategy_crs.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ func (s crsStrategy) ensureCNICRSForCluster(
)
}

if err := client.ServerSideApply(ctx, s.client, cm); err != nil {
if err := client.ServerSideApply(ctx, s.client, cm, client.ForceOwnership); err != nil {
return fmt.Errorf(
"failed to apply Calico CNI installation manifests ConfigMap: %w",
err,
Expand Down Expand Up @@ -185,7 +185,7 @@ func (s crsStrategy) ensureTigeraOperatorConfigMap(
}

tigeraConfigMap := generateTigeraOperatorConfigMap(defaultTigeraOperatorConfigMap, cluster)
if err := client.ServerSideApply(ctx, s.client, tigeraConfigMap); err != nil {
if err := client.ServerSideApply(ctx, s.client, tigeraConfigMap, client.ForceOwnership); err != nil {
return nil, fmt.Errorf(
"failed to apply Tigera Operator manifests ConfigMap: %w",
err,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ func (s helmAddonStrategy) apply(
)
}

if err := client.ServerSideApply(ctx, s.client, hcp); err != nil {
if err := client.ServerSideApply(ctx, s.client, hcp, client.ForceOwnership); err != nil {
return fmt.Errorf("failed to apply Calico CNI installation HelmChartProxy: %w", err)
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/handlers/generic/lifecycle/cni/cilium/strategy_crs.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func (s crsStrategy) apply(
BinaryData: defaultCiliumConfigMap.BinaryData,
}

if err := client.ServerSideApply(ctx, s.client, cm); err != nil {
if err := client.ServerSideApply(ctx, s.client, cm, client.ForceOwnership); err != nil {
return fmt.Errorf(
"failed to apply Cilium CNI installation ConfigMap: %w",
err,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func (s helmAddonStrategy) apply(
)
}

if err := client.ServerSideApply(ctx, s.client, hcp); err != nil {
if err := client.ServerSideApply(ctx, s.client, hcp, client.ForceOwnership); err != nil {
return fmt.Errorf("failed to apply Cilium CNI installation HelmChartProxy: %w", err)
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/handlers/generic/lifecycle/csi/aws-ebs/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func (a *AWSEBS) handleCRSApply(ctx context.Context,
}
cluster := req.Cluster
cm := generateAWSEBSCSIConfigMap(awsEBSCSIConfigMap, &cluster)
if err := client.ServerSideApply(ctx, a.client, cm); err != nil {
if err := client.ServerSideApply(ctx, a.client, cm, client.ForceOwnership); err != nil {
return fmt.Errorf(
"failed to apply AWS EBS CSI manifests ConfigMap: %w",
err,
Expand Down
4 changes: 2 additions & 2 deletions pkg/handlers/generic/lifecycle/csi/nutanix-csi/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ func (n *NutanixCSI) handleHelmAddonApply(
)
}

if err = client.ServerSideApply(ctx, n.client, hcp); err != nil {
if err = client.ServerSideApply(ctx, n.client, hcp, client.ForceOwnership); err != nil {
return fmt.Errorf("failed to apply nutanix-csi installation HelmChartProxy: %w", err)
}

Expand Down Expand Up @@ -215,7 +215,7 @@ func (n *NutanixCSI) handleHelmAddonApply(
},
}

if err = client.ServerSideApply(ctx, n.client, snapshotChart); err != nil {
if err = client.ServerSideApply(ctx, n.client, snapshotChart, client.ForceOwnership); err != nil {
return fmt.Errorf(
"failed to apply nutanix-csi-snapshot installation HelmChartProxy: %w",
err,
Expand Down
2 changes: 1 addition & 1 deletion pkg/handlers/generic/lifecycle/nfd/strategy_crs.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func (s crsStrategy) apply(
BinaryData: defaultCM.BinaryData,
}

if err := client.ServerSideApply(ctx, s.client, cm); err != nil {
if err := client.ServerSideApply(ctx, s.client, cm, client.ForceOwnership); err != nil {
return fmt.Errorf(
"failed to apply NFD installation ConfigMap: %w",
err,
Expand Down
2 changes: 1 addition & 1 deletion pkg/handlers/generic/lifecycle/nfd/strategy_helmaddon.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ image:
)
}

if err := client.ServerSideApply(ctx, s.client, hcp); err != nil {
if err := client.ServerSideApply(ctx, s.client, hcp, client.ForceOwnership); err != nil {
return fmt.Errorf("failed to apply NFD installation HelmChartProxy: %w", err)
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/handlers/generic/lifecycle/utils/scs.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func CreateStorageClassOnRemote(
return fmt.Errorf("error creating client for remote cluster: %w", err)
}
for _, sc := range allStorageClasses {
err = client.ServerSideApply(ctx, remoteClient, sc)
err = client.ServerSideApply(ctx, remoteClient, sc, client.ForceOwnership)
if err != nil {
return fmt.Errorf("error creating storage class %v on remote cluster %w", sc, err)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/handlers/generic/lifecycle/utils/secrets.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func CopySecretToRemoteCluster(
return fmt.Errorf("error creating namespace on the remote cluster: %w", err)
}

err = client.ServerSideApply(ctx, remoteClient, credentialsOnRemote)
err = client.ServerSideApply(ctx, remoteClient, credentialsOnRemote, client.ForceOwnership)
if err != nil {
return fmt.Errorf("error creating Secret on the remote cluster: %w", err)
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/handlers/generic/lifecycle/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func EnsureCRSForClusterFromObjects(
return fmt.Errorf("failed to set owner reference: %w", err)
}

err := client.ServerSideApply(ctx, c, crs)
err := client.ServerSideApply(ctx, c, crs, client.ForceOwnership)
if err != nil {
return fmt.Errorf("failed to server side apply %w", err)
}
Expand All @@ -110,7 +110,7 @@ func EnsureNamespace(ctx context.Context, c ctrlclient.Client, name string) erro
return nil
}

err := client.ServerSideApply(ctx, c, ns)
err := client.ServerSideApply(ctx, c, ns, client.ForceOwnership)
if err != nil {
return fmt.Errorf("failed to server side apply %w", err)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ func createSecretIfNeeded(
)
}
if credentialsSecret != nil {
if err := client.ServerSideApply(ctx, c, credentialsSecret); err != nil {
if err := client.ServerSideApply(ctx, c, credentialsSecret, client.ForceOwnership); err != nil {
return fmt.Errorf("failed to apply Image Registry Credentials Secret: %w", err)
}
}
Expand Down

0 comments on commit 36a0662

Please sign in to comment.