Skip to content

Commit 05d4797

Browse files
aali309aali309
committed
formatting && added test
Signed-off-by: Atif Ali <[email protected]>
1 parent 7de2eb5 commit 05d4797

File tree

2 files changed

+183
-2
lines changed

2 files changed

+183
-2
lines changed

‎pkg/sync/sync_context.go

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -966,6 +966,32 @@ func (sc *syncContext) shouldUseServerSideApply(targetObj *unstructured.Unstruct
966966
return sc.serverSideApply || resourceutil.HasAnnotationOption(targetObj, common.AnnotationSyncOptions, common.SyncOptionServerSideApply)
967967
}
968968

969+
func formatValue(v interface{}) string {
970+
if v == nil {
971+
return "<nil>"
972+
}
973+
974+
// Special case for volumeClaimTemplates
975+
if templates, ok := v.([]interface{}); ok {
976+
names := []string{}
977+
for _, t := range templates {
978+
if template, ok := t.(map[string]interface{}); ok {
979+
if metadata, ok := template["metadata"].(map[string]interface{}); ok {
980+
if name, ok := metadata["name"].(string); ok {
981+
names = append(names, name)
982+
}
983+
}
984+
}
985+
}
986+
return fmt.Sprintf("templates: [%s]", strings.Join(names, ", "))
987+
}
988+
if str, ok := v.(string); ok {
989+
return fmt.Sprintf("%q", str)
990+
}
991+
992+
return fmt.Sprintf("%v", v)
993+
}
994+
969995
func (sc *syncContext) applyObject(t *syncTask, dryRun, validate bool) (common.ResultCode, string) {
970996
dryRunStrategy := cmdutil.DryRunNone
971997
if dryRun {
@@ -1029,13 +1055,19 @@ func (sc *syncContext) applyObject(t *syncTask, dryRun, validate bool) (common.R
10291055
if !mutableFields[k] {
10301056
currentVal, exists := currentSpec[k]
10311057
if !exists || !reflect.DeepEqual(currentVal, desiredVal) {
1032-
changes = append(changes, fmt.Sprintf(" %s -> from %v to %v", k, currentVal, desiredVal))
1058+
// Format the values for better readability
1059+
currentStr := formatValue(currentVal)
1060+
desiredStr := formatValue(desiredVal)
1061+
changes = append(changes, fmt.Sprintf(" - %s:\n from: %s\n to: %s",
1062+
k, currentStr, desiredStr))
10331063
}
10341064
}
10351065
}
10361066

10371067
if len(changes) > 0 {
1038-
return common.ResultCodeSyncFailed, fmt.Sprintf("one or more objects failed to apply, reason: attempting to change immutable fields:\n%s\nForbidden: updates to statefulset spec for fields other than 'replicas', 'ordinals', 'template', 'updateStrategy', 'persistentVolumeClaimRetentionPolicy' and 'minReadySeconds' are forbidden",
1068+
// Sort the changes to ensure consistent order
1069+
sort.Strings(changes)
1070+
return common.ResultCodeSyncFailed, fmt.Sprintf("one or more objects failed to apply, reason: attempting to change immutable fields:\n%s\n\nForbidden: updates to statefulset spec for fields other than 'replicas', 'ordinals', 'template', 'updateStrategy', 'persistentVolumeClaimRetentionPolicy' and 'minReadySeconds' are forbidden",
10391071
strings.Join(changes, "\n"))
10401072
}
10411073
}

‎pkg/sync/sync_context_test.go

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2039,3 +2039,152 @@ func TestWaitForCleanUpBeforeNextWave(t *testing.T) {
20392039
assert.Equal(t, synccommon.ResultCodePruned, result[2].Status)
20402040

20412041
}
2042+
2043+
///////////////
2044+
2045+
func TestStatefulSetImmutableFieldErrors(t *testing.T) {
2046+
tests := []struct {
2047+
name string
2048+
currentSpec map[string]interface{}
2049+
desiredSpec map[string]interface{}
2050+
expectedMessage string
2051+
}{
2052+
{
2053+
name: "single field change - serviceName",
2054+
currentSpec: map[string]interface{}{
2055+
"serviceName": "old-svc",
2056+
},
2057+
desiredSpec: map[string]interface{}{
2058+
"serviceName": "new-svc",
2059+
},
2060+
expectedMessage: `one or more objects failed to apply, reason: attempting to change immutable fields:
2061+
- serviceName:
2062+
from: "old-svc"
2063+
to: "new-svc"
2064+
2065+
Forbidden: updates to statefulset spec for fields other than 'replicas', 'ordinals', 'template', 'updateStrategy', 'persistentVolumeClaimRetentionPolicy' and 'minReadySeconds' are forbidden`,
2066+
},
2067+
{
2068+
name: "volumeClaimTemplates change from nil",
2069+
currentSpec: map[string]interface{}{
2070+
"serviceName": "test-svc",
2071+
},
2072+
desiredSpec: map[string]interface{}{
2073+
"serviceName": "test-svc",
2074+
"volumeClaimTemplates": []interface{}{
2075+
map[string]interface{}{
2076+
"metadata": map[string]interface{}{
2077+
"name": "data",
2078+
},
2079+
},
2080+
},
2081+
},
2082+
expectedMessage: `one or more objects failed to apply, reason: attempting to change immutable fields:
2083+
- volumeClaimTemplates:
2084+
from: <nil>
2085+
to: templates: [data]
2086+
2087+
Forbidden: updates to statefulset spec for fields other than 'replicas', 'ordinals', 'template', 'updateStrategy', 'persistentVolumeClaimRetentionPolicy' and 'minReadySeconds' are forbidden`,
2088+
},
2089+
{
2090+
name: "multiple immutable field changes",
2091+
currentSpec: map[string]interface{}{
2092+
"serviceName": "old-svc",
2093+
"podManagementPolicy": "OrderedReady",
2094+
},
2095+
desiredSpec: map[string]interface{}{
2096+
"serviceName": "new-svc",
2097+
"podManagementPolicy": "Parallel",
2098+
},
2099+
expectedMessage: `one or more objects failed to apply, reason: attempting to change immutable fields:
2100+
- podManagementPolicy:
2101+
from: "OrderedReady"
2102+
to: "Parallel"
2103+
- serviceName:
2104+
from: "old-svc"
2105+
to: "new-svc"
2106+
2107+
Forbidden: updates to statefulset spec for fields other than 'replicas', 'ordinals', 'template', 'updateStrategy', 'persistentVolumeClaimRetentionPolicy' and 'minReadySeconds' are forbidden`,
2108+
},
2109+
{
2110+
name: "complex volumeClaimTemplates change",
2111+
currentSpec: map[string]interface{}{
2112+
"volumeClaimTemplates": []interface{}{
2113+
map[string]interface{}{
2114+
"metadata": map[string]interface{}{
2115+
"name": "data1",
2116+
},
2117+
},
2118+
},
2119+
},
2120+
desiredSpec: map[string]interface{}{
2121+
"volumeClaimTemplates": []interface{}{
2122+
map[string]interface{}{
2123+
"metadata": map[string]interface{}{
2124+
"name": "data1",
2125+
},
2126+
},
2127+
map[string]interface{}{
2128+
"metadata": map[string]interface{}{
2129+
"name": "data2",
2130+
},
2131+
},
2132+
},
2133+
},
2134+
expectedMessage: `one or more objects failed to apply, reason: attempting to change immutable fields:
2135+
- volumeClaimTemplates:
2136+
from: templates: [data1]
2137+
to: templates: [data1, data2]
2138+
2139+
Forbidden: updates to statefulset spec for fields other than 'replicas', 'ordinals', 'template', 'updateStrategy', 'persistentVolumeClaimRetentionPolicy' and 'minReadySeconds' are forbidden`,
2140+
},
2141+
}
2142+
2143+
for _, tt := range tests {
2144+
t.Run(tt.name, func(t *testing.T) {
2145+
current := &unstructured.Unstructured{
2146+
Object: map[string]interface{}{
2147+
"apiVersion": "apps/v1",
2148+
"kind": "StatefulSet",
2149+
"metadata": map[string]interface{}{
2150+
"name": "test-statefulset",
2151+
"namespace": "default",
2152+
},
2153+
"spec": tt.currentSpec,
2154+
},
2155+
}
2156+
2157+
desired := &unstructured.Unstructured{
2158+
Object: map[string]interface{}{
2159+
"apiVersion": "apps/v1",
2160+
"kind": "StatefulSet",
2161+
"metadata": map[string]interface{}{
2162+
"name": "test-statefulset",
2163+
"namespace": "default",
2164+
},
2165+
"spec": tt.desiredSpec,
2166+
},
2167+
}
2168+
2169+
task := &syncTask{
2170+
liveObj: current,
2171+
targetObj: desired,
2172+
}
2173+
2174+
sc := newTestSyncCtx(nil)
2175+
2176+
// Mock the resource operations to return immutable field error
2177+
mockResourceOps := &kubetest.MockResourceOps{
2178+
Commands: map[string]kubetest.KubectlOutput{
2179+
"test-statefulset": {
2180+
Err: errors.New("Forbidden: updates to statefulset spec for fields other than 'replicas', 'ordinals', 'template', 'updateStrategy', 'persistentVolumeClaimRetentionPolicy' and 'minReadySeconds' are forbidden"),
2181+
},
2182+
},
2183+
}
2184+
sc.resourceOps = mockResourceOps
2185+
2186+
_, message := sc.applyObject(task, false, false)
2187+
assert.Equal(t, tt.expectedMessage, message)
2188+
})
2189+
}
2190+
}

0 commit comments

Comments
 (0)