Skip to content

Commit

Permalink
fix(override): failed to override finalizers
Browse files Browse the repository at this point in the history
  • Loading branch information
JackZxj committed Sep 19, 2024
1 parent b29bb15 commit cb49360
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 2 deletions.
15 changes: 14 additions & 1 deletion pkg/controllers/override/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,9 @@ func parseOverrides(
)
}

if err = ctrutil.ApplyJSONPatch(helper.sourceObj, currPatches); err != nil {
// Ignore finalizer patches because they depend on objects in the member cluster.
// The finalizer patches will be applied in sync controller.
if err = ctrutil.ApplyJSONPatch(helper.sourceObj, ignoreFinalizerPatches(currPatches)); err != nil {
return nil, err
}
helpers[cluster.Name] = helper
Expand Down Expand Up @@ -342,6 +344,17 @@ func parsePatchesFromOverriders(
return patches, nil
}

func ignoreFinalizerPatches(p fedcorev1a1.OverridePatches) fedcorev1a1.OverridePatches {
newPatches := make(fedcorev1a1.OverridePatches, 0, len(p))
for _, patch := range p {
if strings.HasPrefix(patch.Path, "/metadata/finalizers") {
continue
}
newPatches = append(newPatches, patch)
}
return newPatches
}

func getGVKFromFederatedObject(fedObject fedcorev1a1.GenericFederatedObject) (schema.GroupVersionKind, error) {
if fedObject == nil {
return schema.GroupVersionKind{}, fmt.Errorf("fedObject is nil")
Expand Down
39 changes: 38 additions & 1 deletion pkg/controllers/override/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -829,7 +829,7 @@ func TestParseOverrides(t *testing.T) {
expectedOverridesMap: make(overridesMap),
isErrorExpected: false,
},
"multiple clusters multiple Overrides(image, command, args, envs, annotations, labels, jsonPatch)" +
"multiple clusters multiple Overrides(image, command, args, envs, annotations, labels, jsonPatch with finalizers)" +
"- should return overrides for each cluster in order": {
fedObject: generateFedObjWithPodWithTwoNormalAndTwoInit(
"docker.io/ealen/echo-server:latest@sha256:bbbbf56b44807c64d294e6c8059b479f35350b454492398225034174808d1726"),
Expand Down Expand Up @@ -899,6 +899,13 @@ func TestParseOverrides(t *testing.T) {
Raw: []byte(`"all"`),
},
},
{
Operator: "add",
Path: "/metadata/finalizers/-",
Value: apiextensionsv1.JSON{
Raw: []byte(`"foo.bar.com/test"`),
},
},
},
},
},
Expand Down Expand Up @@ -940,6 +947,13 @@ func TestParseOverrides(t *testing.T) {
Raw: []byte(`"cluster1"`),
},
},
{
Operator: "replace",
Path: "/metadata/finalizers/0",
Value: apiextensionsv1.JSON{
Raw: []byte(`"foo.bar.com/cluster1"`),
},
},
},
},
},
Expand Down Expand Up @@ -978,6 +992,10 @@ func TestParseOverrides(t *testing.T) {
Operator: "remove",
Path: "/metadata/labels/json2",
},
{
Operator: "remove",
Path: "/metadata/finalizers/0",
},
},
},
},
Expand Down Expand Up @@ -1048,6 +1066,11 @@ func TestParseOverrides(t *testing.T) {
Path: "/metadata/labels/json2",
Value: asJSON("all"),
},
{
Op: "add",
Path: "/metadata/finalizers/-",
Value: asJSON("foo.bar.com/test"),
},
// patches from overrideRules which apply to cluster-1
// image
generatePatch(
Expand Down Expand Up @@ -1079,6 +1102,11 @@ func TestParseOverrides(t *testing.T) {
Path: "/metadata/labels/json1",
Value: asJSON("cluster1"),
},
{
Op: "replace",
Path: "/metadata/finalizers/0",
Value: asJSON("foo.bar.com/cluster1"),
},
},
"cluster2": fedcorev1a1.OverridePatches{
// patches from overrideRules which apply to all clusters
Expand Down Expand Up @@ -1131,6 +1159,11 @@ func TestParseOverrides(t *testing.T) {
Path: "/metadata/labels/json2",
Value: asJSON("all"),
},
{
Op: "add",
Path: "/metadata/finalizers/-",
Value: asJSON("foo.bar.com/test"),
},
// patches from overrideRules which apply to cluster-2
generatePatch(
OperatorReplace,
Expand All @@ -1156,6 +1189,10 @@ func TestParseOverrides(t *testing.T) {
Op: "remove",
Path: "/metadata/labels/json2",
},
{
Op: "remove",
Path: "/metadata/finalizers/0",
},
},
},
isErrorExpected: false,
Expand Down

0 comments on commit cb49360

Please sign in to comment.