Skip to content

Commit

Permalink
Merge pull request #336 from qclc/fix-op
Browse files Browse the repository at this point in the history
fix: the previous cop is completely overwritten when binding op
  • Loading branch information
mrlihanbo authored Sep 3, 2024
2 parents 9191612 + f16c725 commit b29bb15
Show file tree
Hide file tree
Showing 15 changed files with 339 additions and 213 deletions.
11 changes: 3 additions & 8 deletions pkg/controllers/override/annotationslabelspatch.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,22 +27,17 @@ import (
)

func parseStringMapOverriders(
fedObject fedcorev1a1.GenericFederatedObject,
helper *helpData,
overriders []fedcorev1a1.StringMapOverrider,
target string,
) (fedcorev1a1.OverridePatches, error) {
if len(overriders) == 0 {
return fedcorev1a1.OverridePatches{}, nil
}

sourceObj, err := fedObject.GetSpec().GetTemplateAsUnstructured()
if err != nil {
return nil, fmt.Errorf("failed to get sourceObj from fedObj: %w", err)
}

// get labels or annotations of sourceObj
mapValue := getLabelsOrAnnotationsFromObject(sourceObj, target)
mapValue := getLabelsOrAnnotationsFromObject(helper.sourceObj, target)

var err error
// apply StringMapOverriders to mapValue
for index := range overriders {
mapValue, err = applyStringMapOverrider(mapValue, &overriders[index])
Expand Down
6 changes: 5 additions & 1 deletion pkg/controllers/override/annotationslabelspatch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,11 @@ func Test_parseAnnotationsOrLabelsOverriders(t *testing.T) {
testCases := generateTestCases(testTarget)
for testName, testCase := range testCases {
t.Run(testName, func(t *testing.T) {
overridePatches, err := parseStringMapOverriders(testCase.fedObject, testCase.stringMapOverriders, testTarget)
helper, err := getHelpDataFromFedObj(testCase.fedObject)
if err != nil {
t.Fatalf("failed to construct helper: %v", err)
}
overridePatches, err := parseStringMapOverriders(helper, testCase.stringMapOverriders, testTarget)
if (err != nil) != testCase.isErrorExpected {
t.Fatalf("err = %v, but testCase.isErrorExpected = %v", err, testCase.isErrorExpected)
}
Expand Down
12 changes: 4 additions & 8 deletions pkg/controllers/override/commandargspatch.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import (

// parseEntrypointOverriders parse OverridePatches from command/args overriders
func parseEntrypointOverriders(
fedObject fedcorev1a1.GenericFederatedObject,
helper *helpData,
overriders []fedcorev1a1.EntrypointOverrider,
target string,
) (fedcorev1a1.OverridePatches, error) {
Expand All @@ -42,12 +42,8 @@ func parseEntrypointOverriders(
// patchMap(<targetPath, []target]>) is used to store the newest command/args values of each command path
patchMap := make(map[string][]string)

// get gvk from fedObj
gvk, err := getGVKFromFederatedObject(fedObject)
if err != nil {
return nil, err
}
podSpec, err := podutil.GetResourcePodSpec(fedObject, gvk)
// get podSpec from sourceObj
podSpec, err := podutil.GetResourcePodSpecFromUnstructuredObj(helper.sourceObj, helper.gvk)
if err != nil {
return nil, fmt.Errorf("failed to get podSpec from sourceObj: %w", err)
}
Expand All @@ -61,7 +57,7 @@ func parseEntrypointOverriders(
continue
}

targetPath, err := generateTargetPathForPodSpec(gvk, containerKind, target, containerIndex)
targetPath, err := generateTargetPathForPodSpec(helper.gvk, containerKind, target, containerIndex)
if err != nil {
return nil, err
}
Expand Down
6 changes: 5 additions & 1 deletion pkg/controllers/override/commandargspatch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -646,7 +646,11 @@ func Test_parseCommandOrArgsOverriders(t *testing.T) {
for _, testTarget := range testTargets {
for testName, testCase := range generateTestCases(testTarget) {
t.Run(testName, func(t *testing.T) {
overridePatches, err := parseEntrypointOverriders(testCase.fedObject, testCase.entrypointOverriders, testTarget)
helper, err := getHelpDataFromFedObj(testCase.fedObject)
if err != nil {
t.Fatalf("failed to construct helper: %v", err)
}
overridePatches, err := parseEntrypointOverriders(helper, testCase.entrypointOverriders, testTarget)
if (err != nil) != testCase.isErrorExpected {
t.Fatalf("err = %v, but testCase.isErrorExpected = %v", err, testCase.isErrorExpected)
}
Expand Down
11 changes: 3 additions & 8 deletions pkg/controllers/override/envspatch.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import (
)

func parseEnvOverriders(
fedObject fedcorev1a1.GenericFederatedObject,
helper *helpData,
overriders []fedcorev1a1.EnvOverrider,
) (fedcorev1a1.OverridePatches, error) {
if len(overriders) == 0 {
Expand All @@ -39,12 +39,7 @@ func parseEnvOverriders(
// patchMap(<targetPath, []target]>) is used to store the newest env values of each command path
patchMap := make(map[string][]corev1.EnvVar)

// get gvk from fedObj
gvk, err := getGVKFromFederatedObject(fedObject)
if err != nil {
return nil, err
}
podSpec, err := podutil.GetResourcePodSpec(fedObject, gvk)
podSpec, err := podutil.GetResourcePodSpecFromUnstructuredObj(helper.sourceObj, helper.gvk)
if err != nil {
return nil, fmt.Errorf("failed to get podSpec from sourceObj: %w", err)
}
Expand All @@ -58,7 +53,7 @@ func parseEnvOverriders(
continue
}

targetPath, err := generateTargetPathForPodSpec(gvk, containerKind, EnvTarget, containerIndex)
targetPath, err := generateTargetPathForPodSpec(helper.gvk, containerKind, EnvTarget, containerIndex)
if err != nil {
return nil, err
}
Expand Down
6 changes: 5 additions & 1 deletion pkg/controllers/override/envspatch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1089,7 +1089,11 @@ func Test_parseEnvOverriders(t *testing.T) {

for testName, testCase := range generateTestCases() {
t.Run(testName, func(t *testing.T) {
overridePatches, err := parseEnvOverriders(testCase.fedObject, testCase.envOverriders)
helper, err := getHelpDataFromFedObj(testCase.fedObject)
if err != nil {
t.Fatalf("failed to construct helper: %v", err)
}
overridePatches, err := parseEnvOverriders(helper, testCase.envOverriders)
if (err != nil) != testCase.isErrorExpected {
t.Fatalf("err = %v, but testCase.isErrorExpected = %v", err, testCase.isErrorExpected)
}
Expand Down
27 changes: 9 additions & 18 deletions pkg/controllers/override/imagepatch.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ const (
)

func parseImageOverriders(
fedObject fedcorev1a1.GenericFederatedObject,
helper *helpData,
imageOverriders []fedcorev1a1.ImageOverrider,
) (fedcorev1a1.OverridePatches, error) {
// patchMap(<imagePath, imageValue>) is used to store the newest image value of each image path
Expand All @@ -49,11 +49,11 @@ func parseImageOverriders(
for i := range imageOverriders {
imageOverrider := &imageOverriders[i]
if imageOverrider.ImagePath != "" {
if err := parsePatchesFromImagePath(fedObject, imageOverrider, patchMap); err != nil {
if err := parsePatchesFromImagePath(helper, imageOverrider, patchMap); err != nil {
return nil, err
}
} else {
if err := parsePatchesFromWorkload(fedObject, imageOverrider, patchMap); err != nil {
if err := parsePatchesFromWorkload(helper, imageOverrider, patchMap); err != nil {
return nil, err
}
}
Expand Down Expand Up @@ -83,10 +83,10 @@ func parseImageOverriders(
// parsePatchesFromImagePath applies overrider to image value on the specified path,
// and generates a patch which is stored in patchMap
func parsePatchesFromImagePath(
fedObject fedcorev1a1.GenericFederatedObject,
helper *helpData,
imageOverrider *fedcorev1a1.ImageOverrider,
patchMap map[string]string,
) error {
) (err error) {
imagePath := imageOverrider.ImagePath
if !strings.HasPrefix(imagePath, pathSeparator) {
return fmt.Errorf("image path should be start with %s", pathSeparator)
Expand All @@ -95,13 +95,7 @@ func parsePatchesFromImagePath(

// get image value
if imageValue == "" {
// get source obj
sourceObj, err := fedObject.GetSpec().GetTemplateAsUnstructured()
if err != nil {
return fmt.Errorf("failed to get sourceObj from fedObj: %w", err)
}

imageValue, err = GetStringFromUnstructuredObj(sourceObj, imageOverrider.ImagePath)
imageValue, err = GetStringFromUnstructuredObj(helper.sourceObj, imageOverrider.ImagePath)
if err != nil {
return fmt.Errorf("failed to parse image value from unstructured obj: %w", err)
}
Expand All @@ -120,16 +114,13 @@ func parsePatchesFromImagePath(
// parsePatchesFromWorkload applies overrider to image value on the default path of workload,
// and generates a patch which is stored in patchMap
func parsePatchesFromWorkload(
fedObject fedcorev1a1.GenericFederatedObject,
helper *helpData,
imageOverrider *fedcorev1a1.ImageOverrider,
patchMap map[string]string,
) error {
// get pod spec from fedObj
gvk, err := getGVKFromFederatedObject(fedObject)
if err != nil {
return err
}
podSpec, err := podutil.GetResourcePodSpec(fedObject, gvk)
gvk := helper.gvk
podSpec, err := podutil.GetResourcePodSpecFromUnstructuredObj(helper.sourceObj, gvk)
if err != nil {
return fmt.Errorf("failed to get podSpec from sourceObj: %w", err)
}
Expand Down
12 changes: 9 additions & 3 deletions pkg/controllers/override/imagepatch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -912,7 +912,11 @@ func Test_parseImageOverriders(t *testing.T) {

for testName, testCase := range testCases {
t.Run(testName, func(t *testing.T) {
overridePatches, err := parseImageOverriders(testCase.fedObject, testCase.imageOverriders)
helper, err := getHelpDataFromFedObj(testCase.fedObject)
if err != nil {
t.Fatalf("failed to construct helper: %v", err)
}
overridePatches, err := parseImageOverriders(helper, testCase.imageOverriders)
if (err != nil) != testCase.isErrorExpected {
t.Fatalf("err = %v, but testCase.isErrorExpected = %v", err, testCase.isErrorExpected)
}
Expand Down Expand Up @@ -948,7 +952,8 @@ var (
"apiVersion": "apps/v1",
"kind": "Deployment",
"metadata": map[string]interface{}{
"name": "deployment-test",
"name": "deployment-test",
"labels": map[string]interface{}{},
},
"spec": map[string]interface{}{
"replicas": int64(1),
Expand Down Expand Up @@ -1036,7 +1041,8 @@ var (
"apiVersion": "v1",
"kind": "Pod",
"metadata": map[string]interface{}{
"name": "pod-test",
"name": "pod-test",
"labels": map[string]interface{}{},
},
"spec": map[string]interface{}{
"containers": []interface{}{},
Expand Down
3 changes: 2 additions & 1 deletion pkg/controllers/override/overridepolicy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -323,9 +323,10 @@ func (c *Controller) reconcile(ctx context.Context, qualifiedName common.Qualifi
}

var overrides, currentOverrides overridesMap
helpers := map[string]*helpData{}
// Apply overrides from each policy in order
for _, policy := range policies {
newOverrides, err := parseOverrides(policy, placedClusters, fedObject)
newOverrides, err := parseOverrides(policy, placedClusters, fedObject, helpers)
if err != nil {
c.eventRecorder.Eventf(
fedObject,
Expand Down
55 changes: 47 additions & 8 deletions pkg/controllers/override/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,13 @@ import (

apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime/schema"

fedcorev1a1 "github.com/kubewharf/kubeadmiral/pkg/apis/core/v1alpha1"
fedcorev1a1listers "github.com/kubewharf/kubeadmiral/pkg/client/listers/core/v1alpha1"
ctrutil "github.com/kubewharf/kubeadmiral/pkg/controllers/util"
"github.com/kubewharf/kubeadmiral/pkg/util/clusterselector"
podutil "github.com/kubewharf/kubeadmiral/pkg/util/pod"
)
Expand Down Expand Up @@ -112,12 +114,36 @@ func lookForMatchedPolicies(
return policies, false, nil
}

type helpData struct {
gvk schema.GroupVersionKind
sourceObj *unstructured.Unstructured
}

func getHelpDataFromFedObj(fedObj fedcorev1a1.GenericFederatedObject) (*helpData, error) {
gvk, err := getGVKFromFederatedObject(fedObj)
if err != nil {
return nil, fmt.Errorf("failed to get gvk from fedObj: %w", err)
}
sourceObj, err := fedObj.GetSpec().GetTemplateAsUnstructured()
if err != nil {
return nil, fmt.Errorf("failed to get sourceObj from fedObj: %w", err)
}
return &helpData{
gvk: gvk,
sourceObj: sourceObj,
}, nil
}

func parseOverrides(
policy fedcorev1a1.GenericOverridePolicy,
clusters []*fedcorev1a1.FederatedCluster,
fedObject fedcorev1a1.GenericFederatedObject,
helpers map[string]*helpData,
) (overridesMap, error) {
overridesMap := make(overridesMap)
if helpers == nil {
helpers = make(map[string]*helpData)
}

for _, cluster := range clusters {
patches := make(fedcorev1a1.OverridePatches, 0)
Expand All @@ -139,7 +165,14 @@ func parseOverrides(
continue
}

currPatches, err := parsePatchesFromOverriders(fedObject, rule.Overriders)
var helper *helpData
if v, ok := helpers[cluster.Name]; ok {
helper = v
} else if helper, err = getHelpDataFromFedObj(fedObject); err != nil {
return nil, err
}

currPatches, err := parsePatchesFromOverriders(helper, rule.Overriders)
if err != nil {
return nil, fmt.Errorf(
"failed to parse patches from policy %q's overrideRules[%v], error: %w",
Expand All @@ -148,6 +181,12 @@ func parseOverrides(
err,
)
}

if err = ctrutil.ApplyJSONPatch(helper.sourceObj, currPatches); err != nil {
return nil, err
}
helpers[cluster.Name] = helper

patches = append(patches, currPatches...)
}

Expand Down Expand Up @@ -241,7 +280,7 @@ func isClusterMatchedByClusterAffinity(
}

func parsePatchesFromOverriders(
fedObject fedcorev1a1.GenericFederatedObject,
helper *helpData,
overriders *fedcorev1a1.Overriders,
) (fedcorev1a1.OverridePatches, error) {
patches := make(fedcorev1a1.OverridePatches, 0)
Expand All @@ -252,42 +291,42 @@ func parsePatchesFromOverriders(
}

// parse patches from image overriders
if imagePatches, err := parseImageOverriders(fedObject, overriders.Image); err != nil {
if imagePatches, err := parseImageOverriders(helper, overriders.Image); err != nil {
return nil, fmt.Errorf("failed to parse image overriders: %w", err)
} else {
patches = append(patches, imagePatches...)
}

// parse patches from command overriders
if commandPatches, err := parseEntrypointOverriders(fedObject, overriders.Command, CommandTarget); err != nil {
if commandPatches, err := parseEntrypointOverriders(helper, overriders.Command, CommandTarget); err != nil {
return nil, fmt.Errorf("failed to parse command overriders: %w", err)
} else {
patches = append(patches, commandPatches...)
}

// parse patches from args overriders
if argsPatches, err := parseEntrypointOverriders(fedObject, overriders.Args, ArgsTarget); err != nil {
if argsPatches, err := parseEntrypointOverriders(helper, overriders.Args, ArgsTarget); err != nil {
return nil, fmt.Errorf("failed to parse args overriders: %w", err)
} else {
patches = append(patches, argsPatches...)
}

// parse patches from env overriders
if envsPatches, err := parseEnvOverriders(fedObject, overriders.Envs); err != nil {
if envsPatches, err := parseEnvOverriders(helper, overriders.Envs); err != nil {
return nil, fmt.Errorf("failed to parse env overriders: %w", err)
} else {
patches = append(patches, envsPatches...)
}

// parse patches from annotation overriders
if annotationsPatches, err := parseStringMapOverriders(fedObject, overriders.Annotations, AnnotationsTarget); err != nil {
if annotationsPatches, err := parseStringMapOverriders(helper, overriders.Annotations, AnnotationsTarget); err != nil {
return nil, fmt.Errorf("failed to parse annotations overriders: %w", err)
} else {
patches = append(patches, annotationsPatches...)
}

// parse patches from labels overriders
if labelsPatches, err := parseStringMapOverriders(fedObject, overriders.Labels, LabelsTarget); err != nil {
if labelsPatches, err := parseStringMapOverriders(helper, overriders.Labels, LabelsTarget); err != nil {
return nil, fmt.Errorf("failed to parse labels overriders: %w", err)
} else {
patches = append(patches, labelsPatches...)
Expand Down
Loading

0 comments on commit b29bb15

Please sign in to comment.