Skip to content

Commit

Permalink
fix: remove the auto generated selectors for job (#458)
Browse files Browse the repository at this point in the history
  • Loading branch information
@ryanzhang-oss
ryanzhang-oss authored Jul 31, 2023
1 parent 68cac9f commit 681ba86
Show file tree
Hide file tree
Showing 2 changed files with 271 additions and 0 deletions.
13 changes: 13 additions & 0 deletions pkg/controllers/clusterresourceplacement/resource_selector.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"sort"
"strings"

batchv1 "k8s.io/api/batch/v1"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -353,6 +354,18 @@ func generateRawContent(object *unstructured.Unstructured) ([]byte, error) {
if err != nil {
return nil, fmt.Errorf("failed to get the ports field in Serivce object, name =%s: %w", object.GetName(), err)
}
} else if object.GetKind() == "Job" && object.GetAPIVersion() == batchv1.SchemeGroupVersion.String() {
if manualSelector, exist, _ := unstructured.NestedBool(object.Object, "spec", "manualSelector"); !exist || !manualSelector {
// remove the selector field and labels added by the api-server if the job is not created with manual selector
// whose value conflict with the ones created by the member cluster api server
// https://github.com/kubernetes/kubernetes/blob/d4fde1e92a83cb533ae63b3abe9d49f08efb7a2f/pkg/registry/batch/job/strategy.go#L219
// k8s used to add an old label called "controller-uid" but use a new label called "batch.kubernetes.io/controller-uid" after 1.26
unstructured.RemoveNestedField(object.Object, "spec", "selector", "matchLabels", "controller-uid")
unstructured.RemoveNestedField(object.Object, "spec", "selector", "matchLabels", "batch.kubernetes.io/controller-uid")
unstructured.RemoveNestedField(object.Object, "spec", "template", "metadata", "creationTimestamp")
unstructured.RemoveNestedField(object.Object, "spec", "template", "metadata", "labels", "controller-uid")
unstructured.RemoveNestedField(object.Object, "spec", "template", "metadata", "labels", "batch.kubernetes.io/controller-uid")
}
}

rawContent, err := object.MarshalJSON()
Expand Down
258 changes: 258 additions & 0 deletions pkg/controllers/clusterresourceplacement/resource_selector_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

"github.com/google/go-cmp/cmp"
"github.com/stretchr/testify/assert"
batchv1 "k8s.io/api/batch/v1"
corev1 "k8s.io/api/core/v1"
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -280,6 +281,263 @@ func TestGenerateManifest(t *testing.T) {
},
expectedError: nil,
},
"should generate sanitized manifest for Kind: Job": {
// Test that we remove the automatically generated select and labels
unstructuredObj: func() *unstructured.Unstructured {
indexedCompletion := batchv1.IndexedCompletion
job := batchv1.Job{
TypeMeta: metav1.TypeMeta{
APIVersion: "batch/v1",
Kind: "Job",
},
ObjectMeta: metav1.ObjectMeta{
Name: "ryan-name",
Namespace: "ryan-namespace",
DeletionTimestamp: &metav1.Time{Time: time.Date(00002, time.January, 1, 1, 1, 1, 1, time.UTC)},
ManagedFields: []metav1.ManagedFieldsEntry{
{
Manager: "svc-manager",
Operation: metav1.ManagedFieldsOperationApply,
APIVersion: "svc-manager-api/v1",
},
},
OwnerReferences: []metav1.OwnerReference{
{
APIVersion: "svc-ownerRef-api/v1",
Kind: "svc-owner-kind",
Name: "svc-owner-name",
UID: "svc-owner-uid",
},
},
Annotations: map[string]string{
corev1.LastAppliedConfigAnnotation: "svc-object-annotation-lac-value",
"svc-annotation-key": "svc-object-annotation-key-value",
},
ResourceVersion: "svc-object-resourceVersion",
Generation: int64(utilrand.Int()),
CreationTimestamp: metav1.Time{Time: time.Date(00001, time.January, 1, 1, 1, 1, 1, time.UTC)},
UID: types.UID(utilrand.String(10)),
},
Spec: batchv1.JobSpec{
BackoffLimit: pointer.Int32(5),
CompletionMode: &indexedCompletion,
Selector: &metav1.LabelSelector{
MatchLabels: map[string]string{
"foo": "bar",
"job-name": "ryan-name",
"controller-uid": utilrand.String(10),
"batch.kubernetes.io/controller-uid": utilrand.String(10),
},
},
Template: corev1.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{
"foo": "bar",
"controller-uid": utilrand.String(10),
"batch.kubernetes.io/controller-uid": utilrand.String(10),
"job-name": "ryan-name",
"batch.kubernetes.io/job-name": "ryan-name",
},
},
Spec: corev1.PodSpec{
Containers: []corev1.Container{
{Image: "foo/bar"},
},
},
},
},
Status: batchv1.JobStatus{
Active: 1,
Failed: 3,
UncountedTerminatedPods: &batchv1.UncountedTerminatedPods{},
},
}
mJob, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&job)
if err != nil {
t.Fatalf("ToUnstructured failed: %v", err)
}

return &unstructured.Unstructured{Object: mJob}
},
expectedManifest: func() *workv1alpha1.Manifest {
indexedCompletion := batchv1.IndexedCompletion
job := batchv1.Job{
TypeMeta: metav1.TypeMeta{
APIVersion: "batch/v1",
Kind: "Job",
},
ObjectMeta: metav1.ObjectMeta{
Name: "ryan-name",
Namespace: "ryan-namespace",
Annotations: map[string]string{
"svc-annotation-key": "svc-object-annotation-key-value",
},
},
Spec: batchv1.JobSpec{
BackoffLimit: pointer.Int32(5),
CompletionMode: &indexedCompletion,
Selector: &metav1.LabelSelector{
MatchLabels: map[string]string{
"foo": "bar",
"job-name": "ryan-name",
},
},
Template: corev1.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{
"foo": "bar",
"job-name": "ryan-name",
"batch.kubernetes.io/job-name": "ryan-name",
},
},
Spec: corev1.PodSpec{
Containers: []corev1.Container{
{Image: "foo/bar"},
},
},
},
},
}
mJob, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&job)
if err != nil {
t.Fatalf("ToUnstructured failed: %v", err)
}
unstructured.RemoveNestedField(mJob, "status")
unstructured.RemoveNestedField(mJob, "metadata", "creationTimestamp")
unstructured.RemoveNestedField(mJob, "spec", "template", "metadata", "creationTimestamp")

uJob := unstructured.Unstructured{Object: mJob}
rawJob, err := uJob.MarshalJSON()
if err != nil {
t.Fatalf("MarshalJSON failed: %v", err)
}

return &workv1alpha1.Manifest{
RawExtension: runtime.RawExtension{
Raw: rawJob,
},
}
},
expectedError: nil,
},
"should not touch select for Kind: Job with manualSelector": {
// Test that we remove the automatically generated select and labels
unstructuredObj: func() *unstructured.Unstructured {
indexedCompletion := batchv1.IndexedCompletion
job := batchv1.Job{
TypeMeta: metav1.TypeMeta{
APIVersion: "batch/v1",
Kind: "Job",
},
ObjectMeta: metav1.ObjectMeta{
Name: "ryan-name",
Namespace: "ryan-namespace",
DeletionTimestamp: &metav1.Time{Time: time.Date(00002, time.January, 1, 1, 1, 1, 1, time.UTC)},
ResourceVersion: "svc-object-resourceVersion",
Generation: int64(utilrand.Int()),
CreationTimestamp: metav1.Time{Time: time.Date(00001, time.January, 1, 1, 1, 1, 1, time.UTC)},
UID: types.UID(utilrand.String(10)),
},
Spec: batchv1.JobSpec{
BackoffLimit: pointer.Int32(5),
CompletionMode: &indexedCompletion,
ManualSelector: pointer.Bool(true),
Selector: &metav1.LabelSelector{
MatchLabels: map[string]string{
"foo": "bar",
"controller-uid": "ghjdfhsakdfj7824",
"job-name": "ryan-name",
},
},
Template: corev1.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{
"foo": "bar",
"controller-uid": "ghjdfhsakdfj7824",
"job-name": "ryan-name",
"batch.kubernetes.io/job-name": "ryan-name",
},
},
Spec: corev1.PodSpec{
Containers: []corev1.Container{
{Image: "foo/bar"},
},
},
},
},
Status: batchv1.JobStatus{
Active: 1,
Failed: 3,
UncountedTerminatedPods: &batchv1.UncountedTerminatedPods{},
},
}
mJob, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&job)
if err != nil {
t.Fatalf("ToUnstructured failed: %v", err)
}

return &unstructured.Unstructured{Object: mJob}
},
expectedManifest: func() *workv1alpha1.Manifest {
indexedCompletion := batchv1.IndexedCompletion
job := batchv1.Job{
TypeMeta: metav1.TypeMeta{
APIVersion: "batch/v1",
Kind: "Job",
},
ObjectMeta: metav1.ObjectMeta{
Name: "ryan-name",
Namespace: "ryan-namespace",
},
Spec: batchv1.JobSpec{
BackoffLimit: pointer.Int32(5),
CompletionMode: &indexedCompletion,
ManualSelector: pointer.Bool(true),
Selector: &metav1.LabelSelector{
MatchLabels: map[string]string{
"foo": "bar",
"controller-uid": "ghjdfhsakdfj7824",
"job-name": "ryan-name",
},
},
Template: corev1.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{
"foo": "bar",
"controller-uid": "ghjdfhsakdfj7824",
"job-name": "ryan-name",
"batch.kubernetes.io/job-name": "ryan-name",
},
},
Spec: corev1.PodSpec{
Containers: []corev1.Container{
{Image: "foo/bar"},
},
},
},
},
}
mJob, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&job)
if err != nil {
t.Fatalf("ToUnstructured failed: %v", err)
}
unstructured.RemoveNestedField(mJob, "status")
unstructured.RemoveNestedField(mJob, "metadata", "creationTimestamp")

uJob := unstructured.Unstructured{Object: mJob}
rawJob, err := uJob.MarshalJSON()
if err != nil {
t.Fatalf("MarshalJSON failed: %v", err)
}

return &workv1alpha1.Manifest{
RawExtension: runtime.RawExtension{
Raw: rawJob,
},
}
},
expectedError: nil,
},
}

for testName, tt := range tests {
Expand Down

0 comments on commit 681ba86

Please sign in to comment.