Skip to content
This repository has been archived by the owner on Nov 9, 2022. It is now read-only.

Commit

Permalink
Add merge logic for Job
Browse files Browse the repository at this point in the history
```improvement operator
`gardener-resource-manager` handling for Jobs is now improved.
```

Signed-off-by: ialidzhikov <i.alidjikov@gmail.com>
  • Loading branch information
ialidzhikov committed Jun 20, 2020
1 parent d054ee8 commit 6e40fe5
Show file tree
Hide file tree
Showing 3 changed files with 121 additions and 5 deletions.
4 changes: 2 additions & 2 deletions Dockerfile
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
############# builder-base #############
FROM golang:1.13.5 AS builder-base
FROM golang:1.13.12 AS builder-base

WORKDIR /go/src/github.com/gardener/gardener-resource-manager
COPY . .
Expand All @@ -16,7 +16,7 @@ WORKDIR /go/src/github.com/gardener/gardener-resource-manager
RUN make VERIFY=$VERIFY all

############# base #############
FROM alpine:3.10.3 AS base
FROM alpine:3.11.6 AS base

RUN apk add --update bash curl

Expand Down
33 changes: 30 additions & 3 deletions pkg/controller/managedresources/merger.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,10 @@ package managedresources

import (
appsv1 "k8s.io/api/apps/v1"
batchv1 "k8s.io/api/batch/v1"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/kubernetes/scheme"
)
Expand All @@ -42,6 +44,8 @@ func merge(desired, current *unstructured.Unstructured, forceOverwriteLabels boo
switch current.GroupVersionKind().GroupKind() {
case appsv1.SchemeGroupVersion.WithKind("Deployment").GroupKind():
return mergeDeployment(scheme.Scheme, currentCopy, current)
case batchv1.SchemeGroupVersion.WithKind("Job").GroupKind():
return mergeJob(scheme.Scheme, currentCopy, current)
case appsv1.SchemeGroupVersion.WithKind("StatefulSet").GroupKind():
return mergeStatefulSet(scheme.Scheme, currentCopy, current)
case corev1.SchemeGroupVersion.WithKind("Service").GroupKind():
Expand All @@ -64,7 +68,7 @@ func mergeDeployment(scheme *runtime.Scheme, oldObj, newObj runtime.Object) erro
return err
}

// We do not want to overwrite a Deployment's `.spec.replicas' if the new deployments `.spec.replicas`
// Do not overwrite a Deployment's '.spec.replicas' if the new Deployment's '.spec.replicas'
// field is unset.
if newDeployment.Spec.Replicas == nil {
newDeployment.Spec.Replicas = oldDeployment.Spec.Replicas
Expand All @@ -73,6 +77,29 @@ func mergeDeployment(scheme *runtime.Scheme, oldObj, newObj runtime.Object) erro
return scheme.Convert(newDeployment, newObj, nil)
}

func mergeJob(scheme *runtime.Scheme, oldObj, newObj runtime.Object) error {
oldJob := &batchv1.Job{}
if err := scheme.Convert(oldObj, oldJob, nil); err != nil {
return err
}

newJob := &batchv1.Job{}
if err := scheme.Convert(newObj, newJob, nil); err != nil {
return err
}

// Do not overwrite a Job's '.spec.selector' if the new Jobs's '.spec.selector'
// field is unset.
if newJob.Spec.Selector == nil && oldJob.Spec.Selector != nil {
newJob.Spec.Selector = oldJob.Spec.Selector
}

// Do not overwrite Job managed labels as 'controller-uid' and 'job-name'. '.spec.template' is immutable.
newJob.Spec.Template.Labels = labels.Merge(oldJob.Spec.Template.Labels, newJob.Spec.Template.Labels)

return scheme.Convert(newJob, newObj, nil)
}

func mergeStatefulSet(scheme *runtime.Scheme, oldObj, newObj runtime.Object) error {
oldStatefulSet := &appsv1.StatefulSet{}
if err := scheme.Convert(oldObj, oldStatefulSet, nil); err != nil {
Expand All @@ -84,7 +111,7 @@ func mergeStatefulSet(scheme *runtime.Scheme, oldObj, newObj runtime.Object) err
return err
}

// We do not want to overwrite a StatefulSet's `.spec.replicas' if the new deployments `.spec.replicas`
// Do not overwrite a StatefulSet's '.spec.replicas' if the new StatefulSet's `.spec.replicas'
// field is unset.
if newStatefulSet.Spec.Replicas == nil {
newStatefulSet.Spec.Replicas = oldStatefulSet.Spec.Replicas
Expand Down Expand Up @@ -135,7 +162,7 @@ func mergeServiceAccount(scheme *runtime.Scheme, oldObj, newObj runtime.Object)
return err
}

// We do not want to overwrite a ServiceAccount's `.secrets[]` list or `.imagePullSecrets[]`.
// Do not overwrite a ServiceAccount's '.secrets[]' list or '.imagePullSecrets[]'.
newServiceAccount.Secrets = oldServiceAccount.Secrets
newServiceAccount.ImagePullSecrets = oldServiceAccount.ImagePullSecrets

Expand Down
89 changes: 89 additions & 0 deletions pkg/controller/managedresources/merger_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
// Copyright (c) 2020 SAP SE or an SAP affiliate company. All rights reserved. This file is licensed under the Apache Software License, v. 2 except as noted otherwise in the LICENSE file
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package managedresources

import (
batchv1 "k8s.io/api/batch/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"

. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
)

var _ = Describe("merger", func() {

Describe("#mergeJob", func() {
var (
old, new *batchv1.Job
s *runtime.Scheme
)

BeforeEach(func() {
s = runtime.NewScheme()
Expect(batchv1.AddToScheme(s)).ToNot(HaveOccurred(), "schema add should succeed")

old = &batchv1.Job{
ObjectMeta: metav1.ObjectMeta{},
Spec: batchv1.JobSpec{
Selector: &metav1.LabelSelector{
MatchLabels: map[string]string{"controller-uid": "1a2b3c"},
},
Template: corev1.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{"controller-uid": "1a2b3c", "job-name": "pi"},
},
},
},
}

new = old.DeepCopy()
})

It("should not overwrite old .spec.selector if the new one is nil", func() {
new.Spec.Selector = nil

expected := old.DeepCopy()

Expect(mergeJob(s, old, new)).NotTo(HaveOccurred(), "merge should be successful")
Expect(new).To(Equal(expected))
})

It("should not overwrite old .spec.template.labels if the new one is nil", func() {
new.Spec.Template.Labels = nil

expected := old.DeepCopy()

Expect(mergeJob(s, old, new)).NotTo(HaveOccurred(), "merge should be successful")
Expect(new).To(Equal(expected))
})

It("should be able to merge new .spec.template.labels with the old ones", func() {
new.Spec.Template.Labels = map[string]string{"app": "myapp", "version": "v0.1.0"}

expected := old.DeepCopy()
expected.Spec.Template.Labels = map[string]string{
"app": "myapp",
"controller-uid": "1a2b3c",
"job-name": "pi",
"version": "v0.1.0",
}

Expect(mergeJob(s, old, new)).NotTo(HaveOccurred(), "merge should be successful")
Expect(new).To(Equal(expected))
})
})
})

0 comments on commit 6e40fe5

Please sign in to comment.