Skip to content

Commit

Permalink
Fix admission issue
Browse files Browse the repository at this point in the history
  • Loading branch information
TommyLike committed Mar 25, 2019
1 parent baf5dbf commit a602666
Show file tree
Hide file tree
Showing 8 changed files with 15 additions and 56 deletions.
6 changes: 4 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
BIN_DIR=_output/bin
export IMAGE=volcano
export TAG = 1.0
IMAGE=volcano
TAG = 1.0

.EXPORT_ALL_VARIABLES:

all: controllers scheduler cli admission

Expand Down
6 changes: 4 additions & 2 deletions hack/run-e2e-kind.sh
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ export VK_ROOT=$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )/..
export VK_BIN=${VK_ROOT}/_output/bin
export LOG_LEVEL=3
export SHOW_VOLCANO_LOGS=${SHOW_VOLCANO_LOGS:-1}
export CLEANUP_CLUSTER=${CLEANUP_CLUSTER:-1}

if [[ "${CLUSTER_NAME}xxx" != "xxx" ]];then
export CLUSTER_CONTEXT="--name ${CLUSTER_NAME}"
Expand Down Expand Up @@ -98,8 +99,9 @@ Disable displaying volcano component logs:
exit 0
fi


trap cleanup EXIT
if [[ $CLEANUP_CLUSTER -eq 1 ]]; then
trap cleanup EXIT
fi


kind-up-cluster
Expand Down
11 changes: 0 additions & 11 deletions pkg/admission/mutate_job.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ func MutateJobs(ar v1beta1.AdmissionReview) *v1beta1.AdmissionResponse {

func createPatch(job v1alpha1.Job) ([]byte, error) {
var patch []patchOperation
patch = append(patch, mutateJobVersion(job.Status, "/status")...)
patch = append(patch, mutateSpec(job.Spec.Tasks, "/spec/tasks")...)
patch = append(patch, mutateMetadata(job.ObjectMeta, "/metadata")...)

Expand All @@ -96,16 +95,6 @@ func mutateSpec(tasks []v1alpha1.TaskSpec, basePath string) (patch []patchOperat
return patch
}

func mutateJobVersion(jobStatus v1alpha1.JobStatus, basePath string) (patch []patchOperation) {
jobStatus.Version = 1
patch = append(patch, patchOperation{
Op: "replace",
Path: basePath,
Value: jobStatus,
})
return patch
}

func mutateMetadata(metadata metav1.ObjectMeta, basePath string) (patch []patchOperation) {
if len(metadata.Annotations) == 0 {
metadata.Annotations = make(map[string]string)
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/batch/v1alpha1/job.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ type JobStatus struct {
// +optional
Terminating int32 `json:"terminating,omitempty" protobuf:"bytes,7,opt,name=terminating"`
//Current version of job
Version int32
Version int32 `json:"version,omitempty" protobuf:"bytes,8,opt,name=version"`
}

// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
Expand Down
10 changes: 5 additions & 5 deletions pkg/controllers/job/job_controller_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,18 +78,18 @@ func (cc *Controller) updateJob(oldObj, newObj interface{}) {
return
}

if err := cc.cache.Update(newJob); err != nil {
glog.Errorf("Failed to update job <%s/%s>: %v in cache",
newJob.Namespace, newJob.Name, err)
}

//NOTE: Since we only reconcile job based on Spec, we will ignore other attributes
// For Job status, it's used internally and always been updated via our controller.
if reflect.DeepEqual(newJob.Spec, oldJob.Spec) {
glog.Infof("Job update event is ignored since no update in 'Spec'.")
return
}

if err := cc.cache.Update(newJob); err != nil {
glog.Errorf("Failed to update job <%s/%s>: %v in cache",
newJob.Namespace, newJob.Name, err)
}

req := apis.Request{
Namespace: newJob.Namespace,
JobName: newJob.Name,
Expand Down
2 changes: 1 addition & 1 deletion pkg/controllers/job/job_controller_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ func applyPolicies(job *vkv1.Job, req *apis.Request) vkv1.Action {
}

//For all the requests triggered from discarded job resources will perform sync action instead
if req.JobVersion > 0 && req.JobVersion < job.Status.Version {
if req.JobVersion < job.Status.Version {
glog.Infof("Request %s is outdated, will perform sync instead.", req)
return vkv1.SyncJobAction
}
Expand Down
29 changes: 0 additions & 29 deletions test/e2e/admission.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/apis/meta/v1"
"volcano.sh/volcano/pkg/apis/batch/v1alpha1"
)

Expand Down Expand Up @@ -123,32 +122,4 @@ var _ = Describe("Job E2E Test: Test Admission service", func() {
Expect(stError.ErrStatus.Code).To(Equal(int32(500)))
Expect(stError.ErrStatus.Message).To(ContainSubstring("'minAvailable' should not be greater than total replicas in tasks"))
})

It("Job version illegal", func() {
jobName := "job-version-illegal"
namespace := "test"
context := initTestContext()
defer cleanupTestContext(context)
rep := clusterSize(context, oneCPU)

_, err := createJobInner(context, &jobSpec{
version: 12,
namespace: namespace,
name: jobName,
tasks: []taskSpec{
{
img: defaultNginxImage,
req: oneCPU,
min: rep,
rep: rep,
name: "taskname",
},
},
})
Expect(err).NotTo(HaveOccurred())
job, err := context.vkclient.BatchV1alpha1().Jobs("test").Get(jobName, v1.GetOptions{})
Expect(err).NotTo(HaveOccurred())
//Job version are always set to 1
Expect(job.Status.Version).To(Equal(int32(1)))
})
})
5 changes: 0 additions & 5 deletions test/e2e/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,6 @@ type jobSpec struct {
tasks []taskSpec
policies []vkv1.LifecyclePolicy
min int32
version int32
}

func getNS(context *context, job *jobSpec) string {
Expand Down Expand Up @@ -376,10 +375,6 @@ func createJobInner(context *context, jobSpec *jobSpec) (*vkv1.Job, error) {
min += task.min
}

if jobSpec.version > 0 {
job.Status.Version = jobSpec.version
}

if jobSpec.min > 0 {
job.Spec.MinAvailable = jobSpec.min
} else {
Expand Down

0 comments on commit a602666

Please sign in to comment.