Skip to content

Commit 4487374

Browse files
alloydsaalloydsa
authored
Fix preserving annotation of existing deployment during trident upgrade
1 parent aafa114 commit 4487374

File tree

2 files changed

+320
-2
lines changed

2 files changed

+320
-2
lines changed

‎operator/controllers/orchestrator/installer/k8s_client.go‎

Lines changed: 46 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1415,11 +1415,17 @@ func (k *K8sClient) PutDeployment(
14151415
"namespace": k.Namespace(),
14161416
}
14171417

1418+
// Preserve annotations from current deployment
1419+
updatedDeploymentYAML, err := mergeAnnotationsFromExistingDeployment(currentDeployment, newDeploymentYAML)
1420+
if err != nil {
1421+
return fmt.Errorf("could not merge annotations from current deployment %q: %v", deploymentName, err)
1422+
}
1423+
14181424
if createDeployment {
14191425
Log().WithFields(logFields).Debug("Creating Trident deployment.")
14201426

14211427
// Create the deployment
1422-
if err := k.CreateObjectByYAML(newDeploymentYAML); err != nil {
1428+
if err := k.CreateObjectByYAML(updatedDeploymentYAML); err != nil {
14231429
return fmt.Errorf("could not create Trident deployment; %v", err)
14241430
}
14251431

@@ -1428,7 +1434,7 @@ func (k *K8sClient) PutDeployment(
14281434
Log().WithFields(logFields).Debug("Patching Trident deployment.")
14291435

14301436
// Identify the deltas
1431-
patchBytes, err := k8sclient.GenericPatch(currentDeployment, []byte(newDeploymentYAML))
1437+
patchBytes, err := k8sclient.GenericPatch(currentDeployment, []byte(updatedDeploymentYAML))
14321438
if err != nil {
14331439
return fmt.Errorf("error in creating the two-way merge patch for current Deployment %q: %v",
14341440
deploymentName, err)
@@ -2335,3 +2341,41 @@ func (k *K8sClient) ExecPodForVersionInformation(podName string, cmd []string, t
23352341

23362342
return execOutput, nil
23372343
}
2344+
2345+
// mergeAnnotationsFromExistingDeployment preserves annotations from an existing deployment into a new deployment
2346+
// during trident-controller upgrade.
2347+
func mergeAnnotationsFromExistingDeployment(existingDeployment *appsv1.Deployment, newDeploymentYAML string) (string,
2348+
error) {
2349+
if existingDeployment == nil {
2350+
return newDeploymentYAML, nil
2351+
}
2352+
2353+
existingAnnotations := existingDeployment.GetAnnotations()
2354+
if len(existingAnnotations) == 0 {
2355+
return newDeploymentYAML, nil
2356+
}
2357+
2358+
// Parse new deployment YAML into deployment object
2359+
var newDeployment appsv1.Deployment
2360+
if err := yaml.Unmarshal([]byte(newDeploymentYAML), &newDeployment); err != nil {
2361+
return "", fmt.Errorf("failed to unmarshal new deployment yaml: %w", err)
2362+
}
2363+
2364+
if newDeployment.Annotations == nil {
2365+
newDeployment.Annotations = existingAnnotations
2366+
} else {
2367+
for annotationKey, annotationValue := range existingAnnotations {
2368+
if _, exists := newDeployment.Annotations[annotationKey]; !exists {
2369+
newDeployment.Annotations[annotationKey] = annotationValue
2370+
}
2371+
}
2372+
}
2373+
2374+
// Convert updated new deployment object back to YAML
2375+
updatedNewDeploymentYAMLBytes, err := yaml.Marshal(newDeployment)
2376+
if err != nil {
2377+
return "", fmt.Errorf("failed to marshal updated deployment: %w", err)
2378+
}
2379+
2380+
return string(updatedNewDeploymentYAMLBytes), nil
2381+
}

‎operator/controllers/orchestrator/installer/k8s_client_test.go‎

Lines changed: 274 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6519,3 +6519,277 @@ func TestNewExtendedK8sClient(t *testing.T) {
65196519
}
65206520
})
65216521
}
6522+
6523+
func TestMergeAnnotationsFromExistingDeployment(t *testing.T) {
6524+
// arrange variables for the tests
6525+
newDeploymentYAML := `apiVersion: apps/v1
6526+
kind: Deployment
6527+
metadata:
6528+
name: trident-controller
6529+
namespace: trident
6530+
annotations:
6531+
existing-annotation: "existing-value"
6532+
spec:
6533+
replicas: 1
6534+
selector:
6535+
matchLabels:
6536+
app: controller.csi.trident.netapp.io
6537+
template:
6538+
metadata:
6539+
labels:
6540+
app: controller.csi.trident.netapp.io
6541+
spec:
6542+
containers:
6543+
- name: trident-main
6544+
image: netapp/trident:v25.01.0
6545+
`
6546+
6547+
existingDeploymentWithAnnotations := &appsv1.Deployment{
6548+
ObjectMeta: metav1.ObjectMeta{
6549+
Name: "trident-controller",
6550+
Namespace: "trident",
6551+
Annotations: map[string]string{
6552+
"old-annotation-1": "old-value-1",
6553+
"old-annotation-2": "old-value-2",
6554+
"existing-annotation": "should-not-override",
6555+
},
6556+
},
6557+
}
6558+
6559+
existingDeploymentNoAnnotations := &appsv1.Deployment{
6560+
ObjectMeta: metav1.ObjectMeta{
6561+
Name: "trident-controller",
6562+
Namespace: "trident",
6563+
},
6564+
}
6565+
6566+
invalidYAML := `invalid: yaml: content: [unclosed`
6567+
6568+
type input struct {
6569+
existingDeployment *appsv1.Deployment
6570+
newDeploymentYAML string
6571+
}
6572+
6573+
tests := map[string]struct {
6574+
input input
6575+
expectError bool
6576+
validate func(t *testing.T, result string, err error)
6577+
}{
6578+
"nil existing deployment returns unchanged YAML": {
6579+
input: input{
6580+
existingDeployment: nil,
6581+
newDeploymentYAML: newDeploymentYAML,
6582+
},
6583+
expectError: false,
6584+
validate: func(t *testing.T, result string, err error) {
6585+
assert.NoError(t, err)
6586+
assert.Equal(t, newDeploymentYAML, result)
6587+
},
6588+
},
6589+
"existing deployment with no annotations returns unchanged YAML": {
6590+
input: input{
6591+
existingDeployment: existingDeploymentNoAnnotations,
6592+
newDeploymentYAML: newDeploymentYAML,
6593+
},
6594+
expectError: false,
6595+
validate: func(t *testing.T, result string, err error) {
6596+
assert.NoError(t, err)
6597+
assert.Equal(t, newDeploymentYAML, result)
6598+
},
6599+
},
6600+
"existing deployment with annotations merges non-conflicting annotations": {
6601+
input: input{
6602+
existingDeployment: existingDeploymentWithAnnotations,
6603+
newDeploymentYAML: newDeploymentYAML,
6604+
},
6605+
expectError: false,
6606+
validate: func(t *testing.T, result string, err error) {
6607+
assert.NoError(t, err)
6608+
assert.NotEqual(t, newDeploymentYAML, result)
6609+
6610+
// Parse the result to verify annotations are merged
6611+
var updatedDeployment appsv1.Deployment
6612+
unmarshalErr := yaml.Unmarshal([]byte(result), &updatedDeployment)
6613+
assert.NoError(t, unmarshalErr)
6614+
6615+
// Verify old annotations are added
6616+
assert.Equal(t, "old-value-1", updatedDeployment.Annotations["old-annotation-1"])
6617+
assert.Equal(t, "old-value-2", updatedDeployment.Annotations["old-annotation-2"])
6618+
6619+
// Verify existing annotation is NOT overridden (new deployment value should be preserved)
6620+
assert.Equal(t, "existing-value", updatedDeployment.Annotations["existing-annotation"])
6621+
6622+
// Verify total annotation count
6623+
assert.Equal(t, 3, len(updatedDeployment.Annotations))
6624+
},
6625+
},
6626+
"new deployment without annotations gets all existing annotations": {
6627+
input: input{
6628+
existingDeployment: existingDeploymentWithAnnotations,
6629+
newDeploymentYAML: `apiVersion: apps/v1
6630+
kind: Deployment
6631+
metadata:
6632+
name: trident-controller
6633+
namespace: trident
6634+
spec:
6635+
replicas: 1
6636+
`,
6637+
},
6638+
expectError: false,
6639+
validate: func(t *testing.T, result string, err error) {
6640+
assert.NoError(t, err)
6641+
6642+
// Parse the result to verify all existing annotations are added
6643+
var updatedDeployment appsv1.Deployment
6644+
unmarshalErr := yaml.Unmarshal([]byte(result), &updatedDeployment)
6645+
assert.NoError(t, unmarshalErr)
6646+
6647+
// Verify all existing annotations are copied
6648+
assert.Equal(t, "old-value-1", updatedDeployment.Annotations["old-annotation-1"])
6649+
assert.Equal(t, "old-value-2", updatedDeployment.Annotations["old-annotation-2"])
6650+
assert.Equal(t, "should-not-override", updatedDeployment.Annotations["existing-annotation"])
6651+
assert.Equal(t, 3, len(updatedDeployment.Annotations))
6652+
},
6653+
},
6654+
"invalid new deployment YAML returns error": {
6655+
input: input{
6656+
existingDeployment: existingDeploymentWithAnnotations,
6657+
newDeploymentYAML: invalidYAML,
6658+
},
6659+
expectError: true,
6660+
validate: func(t *testing.T, result string, err error) {
6661+
assert.Error(t, err)
6662+
assert.Contains(t, err.Error(), "failed to unmarshal new deployment yaml")
6663+
assert.Empty(t, result)
6664+
},
6665+
},
6666+
"existing deployment with single annotation merges correctly": {
6667+
input: input{
6668+
existingDeployment: &appsv1.Deployment{
6669+
ObjectMeta: metav1.ObjectMeta{
6670+
Name: "trident-controller",
6671+
Namespace: "trident",
6672+
Annotations: map[string]string{
6673+
"single-annotation": "single-value",
6674+
},
6675+
},
6676+
},
6677+
newDeploymentYAML: newDeploymentYAML,
6678+
},
6679+
expectError: false,
6680+
validate: func(t *testing.T, result string, err error) {
6681+
assert.NoError(t, err)
6682+
6683+
var updatedDeployment appsv1.Deployment
6684+
unmarshalErr := yaml.Unmarshal([]byte(result), &updatedDeployment)
6685+
assert.NoError(t, unmarshalErr)
6686+
6687+
assert.Equal(t, "single-value", updatedDeployment.Annotations["single-annotation"])
6688+
assert.Equal(t, "existing-value", updatedDeployment.Annotations["existing-annotation"])
6689+
assert.Equal(t, 2, len(updatedDeployment.Annotations))
6690+
},
6691+
},
6692+
"existing deployment with empty annotation value merges correctly": {
6693+
input: input{
6694+
existingDeployment: &appsv1.Deployment{
6695+
ObjectMeta: metav1.ObjectMeta{
6696+
Name: "trident-controller",
6697+
Namespace: "trident",
6698+
Annotations: map[string]string{
6699+
"empty-annotation": "",
6700+
},
6701+
},
6702+
},
6703+
newDeploymentYAML: newDeploymentYAML,
6704+
},
6705+
expectError: false,
6706+
validate: func(t *testing.T, result string, err error) {
6707+
assert.NoError(t, err)
6708+
6709+
var updatedDeployment appsv1.Deployment
6710+
unmarshalErr := yaml.Unmarshal([]byte(result), &updatedDeployment)
6711+
assert.NoError(t, unmarshalErr)
6712+
6713+
// Empty annotation should still be present
6714+
val, exists := updatedDeployment.Annotations["empty-annotation"]
6715+
assert.True(t, exists)
6716+
assert.Equal(t, "", val)
6717+
assert.Equal(t, 2, len(updatedDeployment.Annotations))
6718+
},
6719+
},
6720+
"existing deployment with special characters in annotations merges correctly": {
6721+
input: input{
6722+
existingDeployment: &appsv1.Deployment{
6723+
ObjectMeta: metav1.ObjectMeta{
6724+
Name: "trident-controller",
6725+
Namespace: "trident",
6726+
Annotations: map[string]string{
6727+
"special/annotation": "value-with-slash",
6728+
"annotation.with.dots": "dotted-value",
6729+
"annotation-with-dashes": "dashed-value",
6730+
"annotation_with_undersc": "underscore-value",
6731+
},
6732+
},
6733+
},
6734+
newDeploymentYAML: newDeploymentYAML,
6735+
},
6736+
expectError: false,
6737+
validate: func(t *testing.T, result string, err error) {
6738+
assert.NoError(t, err)
6739+
6740+
var updatedDeployment appsv1.Deployment
6741+
unmarshalErr := yaml.Unmarshal([]byte(result), &updatedDeployment)
6742+
assert.NoError(t, unmarshalErr)
6743+
6744+
assert.Equal(t, "value-with-slash", updatedDeployment.Annotations["special/annotation"])
6745+
assert.Equal(t, "dotted-value", updatedDeployment.Annotations["annotation.with.dots"])
6746+
assert.Equal(t, "dashed-value", updatedDeployment.Annotations["annotation-with-dashes"])
6747+
assert.Equal(t, "underscore-value", updatedDeployment.Annotations["annotation_with_undersc"])
6748+
assert.Equal(t, 5, len(updatedDeployment.Annotations))
6749+
},
6750+
},
6751+
"multiple merges preserve deployment structure": {
6752+
input: input{
6753+
existingDeployment: existingDeploymentWithAnnotations,
6754+
newDeploymentYAML: newDeploymentYAML,
6755+
},
6756+
expectError: false,
6757+
validate: func(t *testing.T, result string, err error) {
6758+
assert.NoError(t, err)
6759+
6760+
var updatedDeployment appsv1.Deployment
6761+
unmarshalErr := yaml.Unmarshal([]byte(result), &updatedDeployment)
6762+
assert.NoError(t, unmarshalErr)
6763+
6764+
// Verify deployment structure is preserved
6765+
assert.Equal(t, "trident-controller", updatedDeployment.Name)
6766+
assert.Equal(t, "trident", updatedDeployment.Namespace)
6767+
assert.Equal(t, "apps/v1", updatedDeployment.APIVersion)
6768+
assert.Equal(t, "Deployment", updatedDeployment.Kind)
6769+
6770+
// Verify spec is preserved
6771+
assert.NotNil(t, updatedDeployment.Spec)
6772+
assert.Equal(t, int32(1), *updatedDeployment.Spec.Replicas)
6773+
},
6774+
},
6775+
}
6776+
6777+
for testName, test := range tests {
6778+
t.Run(testName, func(t *testing.T) {
6779+
result, err := mergeAnnotationsFromExistingDeployment(
6780+
test.input.existingDeployment,
6781+
test.input.newDeploymentYAML,
6782+
)
6783+
6784+
if test.expectError {
6785+
assert.Error(t, err)
6786+
} else {
6787+
assert.NoError(t, err)
6788+
}
6789+
6790+
if test.validate != nil {
6791+
test.validate(t, result, err)
6792+
}
6793+
})
6794+
}
6795+
}

0 commit comments

Comments
 (0)