You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@yunikorn.apache.org by wi...@apache.org on 2026/09/29 01:43:45 UTC
(yunikorn-k8shim) 02/02: [YUNIKORN-3473] Support workload updates (#1106)
This is an automated email from the ASF dual-hosted git repository.
wilfred-s pushed a commit to branch branch-1.10
in repository https://gitbox.apache.org/repos/asf/yunikorn-k8shim.git
commit 170fa4abc0f62761a331ba20571d6b314d2b727a
Author: Wilfred Spiegelenburg <wi...@apache.org>
AuthorDate: Mon Sep 28 16:58:33 2026 +0530
[YUNIKORN-3473] Support workload updates (#1106)
Application ID and user info checks should be consistent for creates and
updates of a workload. Implement workload updates checks instead of a
blanket approval.
Partial refactor of the extractor code, more to follow in YUNIKORN-3477.
Remove references to the batch/v1beta1 for CronJob which was removed in
K8s 1.25.
Closes: #1106
Signed-off-by: Manikandan R <ma...@gmail.com>
(cherry picked from commit 15fb79ab15ec46b65f5ce7c8d7f178b0e2547ff6)
---
pkg/admission/admission_controller.go | 113 +++++++++----
pkg/admission/admission_controller_test.go | 259 +++++++++++++++++++++++++----
pkg/admission/common/common.go | 8 +-
pkg/admission/metadata/extract.go | 82 ++++++---
pkg/admission/metadata/labels.go | 18 +-
pkg/admission/metadata/labels_test.go | 8 +-
pkg/admission/metadata/usergroup.go | 27 ++-
pkg/admission/metadata/usergroup_test.go | 22 +--
pkg/admission/util.go | 30 +++-
pkg/cache/task.go | 6 +-
10 files changed, 435 insertions(+), 138 deletions(-)
diff --git a/pkg/admission/admission_controller.go b/pkg/admission/admission_controller.go
index 4067fb95..48103716 100644
--- a/pkg/admission/admission_controller.go
+++ b/pkg/admission/admission_controller.go
@@ -30,7 +30,6 @@ import (
"strings"
"go.uber.org/zap"
-
admissionv1 "k8s.io/api/admission/v1"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -144,21 +143,17 @@ func (c *AdmissionController) mutate(req *admissionv1.AdmissionRequest) *admissi
if req.Kind.Kind == metadata.Pod {
return c.processPodUpdate(req, namespace)
}
-
- // resource types other than pods are ignored for UPDATE operations
- return admissionResponseBuilder(string(req.UID), true, "", nil)
+ return c.processWorkloadUpdate(req, namespace)
}
if req.Kind.Kind == metadata.Pod {
return c.processPod(req, namespace)
}
-
return c.processWorkload(req, namespace)
}
func (c *AdmissionController) processPod(req *admissionv1.AdmissionRequest, namespace string) *admissionv1.AdmissionResponse {
- var patch []common.PatchOperation
- var uid = string(req.UID)
+ uid := string(req.UID)
var pod v1.Pod
if err := json.Unmarshal(req.Object.Raw, &pod); err != nil {
@@ -193,6 +188,7 @@ func (c *AdmissionController) processPod(req *admissionv1.AdmissionRequest, name
return failureResponse
}
+ var patch []common.PatchOperation
if !userInfoSet && !c.conf.GetBypassAuth() {
log.Log(log.Admission).Info("setting user info metadata on pod")
patchOp, err := c.annotationHandler.GetPatchForPod(pod.Annotations, userName, groups)
@@ -224,16 +220,13 @@ func (c *AdmissionController) processPod(req *admissionv1.AdmissionRequest, name
}
func (c *AdmissionController) processWorkload(req *admissionv1.AdmissionRequest, namespace string) *admissionv1.AdmissionResponse {
- var uid = string(req.UID)
+ uid := string(req.UID)
if !c.shouldProcessWorkload(req) {
return admissionResponseBuilder(uid, true, "", nil)
}
- var supported bool
- var err error
- var labels map[string]string
- labels, supported, err = c.labelExtractor.GetLabelsFromWorkload(req)
+ labels, supported, err := c.labelExtractor.GetLabelsFromRequest(req, false)
if !supported {
// Unknown request kind - pass
return admissionResponseBuilder(uid, true, "", nil)
@@ -241,22 +234,16 @@ func (c *AdmissionController) processWorkload(req *admissionv1.AdmissionRequest,
if err != nil {
return admissionResponseBuilder(uid, false, err.Error(), nil)
}
+ // this cannot return unsupported or an error as we have already pulled the labels from the same object which
+ // extracts the exact same data returning a different piece.
+ annotations, _, _ := c.annotationHandler.GetAnnotationsFromRequest(req, false) //nolint: errcheck
- if !c.shouldProcessAdmissionReview(namespace, labels) {
+ appID := getApplicationIDValue(labels, annotations)
+ if !c.shouldProcessAdmissionReview(namespace, appID) {
log.Log(log.Admission).Info("bypassing namespace", zap.String("namespace", namespace))
return admissionResponseBuilder(uid, true, "", nil)
}
- var annotations map[string]string
- annotations, supported, err = c.annotationHandler.GetAnnotationsFromRequestKind(req)
- if !supported {
- // Unknown request kind - pass
- return admissionResponseBuilder(uid, true, "", nil)
- }
- if err != nil {
- return admissionResponseBuilder(uid, false, err.Error(), nil)
- }
-
userName := req.UserInfo.Username
groups := req.UserInfo.Groups
failureResponse, userInfoSet := c.checkUserInfoAnnotation(func() (string, bool) {
@@ -268,13 +255,14 @@ func (c *AdmissionController) processWorkload(req *admissionv1.AdmissionRequest,
}
if !userInfoSet && !c.conf.GetBypassAuth() {
- patch, err := c.annotationHandler.GetPatchForWorkload(req, userName, groups)
+ var patch *common.PatchOperation
+ patch, err = c.annotationHandler.GetPatchForWorkload(req, userName, groups)
if err != nil {
log.Log(log.Admission).Error("could not generate patch for workload", zap.Error(err))
return admissionResponseBuilder(uid, false, err.Error(), nil)
}
- patchBytes, patchErr := json.Marshal(patch)
+ patchBytes, patchErr := json.Marshal([]common.PatchOperation{*patch})
if patchErr != nil {
log.Log(log.Admission).Error("failed to marshal patch", zap.Error(patchErr))
return admissionResponseBuilder(uid, false, patchErr.Error(), nil)
@@ -315,15 +303,18 @@ func (c *AdmissionController) processPodUpdate(req *admissionv1.AdmissionRequest
log.Log(log.Admission).Info("Non YuniKorn pod in scheduler namespace", zap.String("UID", uid))
}
- if !c.shouldProcessAdmissionReview(namespace, newPod.Labels) {
- log.Log(log.Admission).Info("pod update - bypassing namespace", zap.String("namespace", namespace))
+ // schedulername can only be set on create if it is not YuniKorn when we get here we should not process it as
+ // the name was not patched
+ if !isScheduledByYuniKorn(&newPod) {
+ log.Log(log.Admission).Info("pod update - not scheduled by YuniKorn", zap.String("namespace", namespace))
return admissionResponseBuilder(uid, true, "", nil)
}
originalUserInfo := oldPod.Annotations[common.UserInfoAnnotation]
newUserInfo := newPod.Annotations[common.UserInfoAnnotation]
- log.Log(log.Admission).Debug("checking original and new pod annotation", zap.String("original", originalUserInfo),
+ log.Log(log.Admission).Debug("checking original and new pod annotation",
+ zap.String("original", originalUserInfo),
zap.String("new", newUserInfo))
if originalUserInfo != newUserInfo {
@@ -338,9 +329,65 @@ func (c *AdmissionController) processPodUpdate(req *admissionv1.AdmissionRequest
return admissionResponseBuilder(uid, true, "", nil)
}
-func (c *AdmissionController) shouldProcessAdmissionReview(namespace string, labels map[string]string) bool {
- if c.shouldProcessNamespace(namespace) &&
- (labels[constants.CanonicalLabelApplicationID] != "" || labels[constants.LabelApplicationID] != "" || labels[constants.SparkLabelAppID] != "" || c.shouldLabelNamespace(namespace)) {
+func (c *AdmissionController) processWorkloadUpdate(req *admissionv1.AdmissionRequest, namespace string) *admissionv1.AdmissionResponse {
+ var uid = string(req.UID)
+
+ if !c.shouldProcessWorkload(req) {
+ return admissionResponseBuilder(uid, true, "", nil)
+ }
+
+ labels, supported, err := c.labelExtractor.GetLabelsFromRequest(req, false)
+ if !supported {
+ // Unknown request kind - pass
+ return admissionResponseBuilder(uid, true, "", nil)
+ }
+ if err != nil {
+ return admissionResponseBuilder(uid, false, err.Error(), nil)
+ }
+
+ var oldLabels map[string]string
+ oldLabels, supported, err = c.labelExtractor.GetLabelsFromRequest(req, true)
+ if !supported {
+ // Unknown request kind - pass
+ return admissionResponseBuilder(uid, true, "", nil)
+ }
+ if err != nil {
+ return admissionResponseBuilder(uid, false, err.Error(), nil)
+ }
+
+ // these cannot fail as we have done both object earlier
+ annotations, _, _ := c.annotationHandler.GetAnnotationsFromRequest(req, false) //nolint: errcheck
+ oldAnnotations, _, _ := c.annotationHandler.GetAnnotationsFromRequest(req, true) //nolint: errcheck
+ oldAppID := getApplicationIDValue(oldLabels, oldAnnotations)
+ newAppID := getApplicationIDValue(labels, annotations)
+ // This might look strange, but we just need to know if old or new appID are not empty.
+ // Both can be empty and the resulting pod could get a generated ID but if set it should be taken into account
+ if !c.shouldProcessAdmissionReview(namespace, oldAppID+newAppID) {
+ log.Log(log.Admission).Info("bypassing namespace", zap.String("namespace", namespace))
+ return admissionResponseBuilder(uid, true, "", nil)
+ }
+ originalUserInfo := oldAnnotations[common.UserInfoAnnotation]
+ newUserInfo := annotations[common.UserInfoAnnotation]
+
+ log.Log(log.Admission).Debug("checking original and new workload user annotations",
+ zap.String("original", originalUserInfo),
+ zap.String("new", newUserInfo))
+
+ if originalUserInfo != newUserInfo {
+ log.Log(log.Admission).Info("workload update - userinfo annotation change not allowed", zap.String("UID", uid))
+ return admissionResponseBuilder(uid, false, "user info annotation change is not allowed", nil)
+ }
+
+ if oldAppID != newAppID {
+ log.Log(log.Admission).Info("workload update - application ID change not allowed", zap.String("UID", uid))
+ return admissionResponseBuilder(uid, false, "applicationID change is not allowed", nil)
+ }
+
+ return admissionResponseBuilder(uid, true, "", nil)
+}
+
+func (c *AdmissionController) shouldProcessAdmissionReview(namespace string, appID string) bool {
+ if c.shouldProcessNamespace(namespace) && (appID != "" || c.shouldLabelNamespace(namespace)) {
return true
}
@@ -415,7 +462,7 @@ func (c *AdmissionController) updatePreemptionInfo(pod *v1.Pod, patch []common.P
// check for an existing patch on annotations and update it
for _, p := range patch {
- if p.Op == "add" && p.Path == "/metadata/annotations" {
+ if p.Op == "add" && p.Path == metadata.PodAnnotationsPath {
if annotations, ok := p.Value.(map[string]string); ok {
annotations[constants.AnnotationAllowPreemption] = value
return patch
@@ -426,7 +473,7 @@ func (c *AdmissionController) updatePreemptionInfo(pod *v1.Pod, patch []common.P
result := updatePodAnnotation(pod, constants.AnnotationAllowPreemption, value)
patch = append(patch, common.PatchOperation{
Op: "add",
- Path: "/metadata/annotations",
+ Path: metadata.PodAnnotationsPath,
Value: result,
})
diff --git a/pkg/admission/admission_controller_test.go b/pkg/admission/admission_controller_test.go
index 6d73828c..5ea7848b 100644
--- a/pkg/admission/admission_controller_test.go
+++ b/pkg/admission/admission_controller_test.go
@@ -28,16 +28,17 @@ import (
"testing"
"gotest.tools/v3/assert"
-
admissionv1 "k8s.io/api/admission/v1"
appsv1 "k8s.io/api/apps/v1"
authv1 "k8s.io/api/authentication/v1"
+ batchv1 "k8s.io/api/batch/v1"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"github.com/apache/yunikorn-k8shim/pkg/admission/common"
"github.com/apache/yunikorn-k8shim/pkg/admission/conf"
+ "github.com/apache/yunikorn-k8shim/pkg/admission/metadata"
"github.com/apache/yunikorn-k8shim/pkg/common/constants"
schedulerconf "github.com/apache/yunikorn-k8shim/pkg/conf"
)
@@ -59,7 +60,7 @@ func TestUpdateLabels(t *testing.T) {
pod := &v1.Pod{
TypeMeta: metav1.TypeMeta{
- Kind: "Pod",
+ Kind: metadata.Pod,
APIVersion: "v1",
},
ObjectMeta: metav1.ObjectMeta{
@@ -96,7 +97,7 @@ func TestUpdateLabels(t *testing.T) {
pod = &v1.Pod{
TypeMeta: metav1.TypeMeta{
- Kind: "Pod",
+ Kind: metadata.Pod,
APIVersion: "v1",
},
ObjectMeta: metav1.ObjectMeta{
@@ -132,7 +133,7 @@ func TestUpdateLabels(t *testing.T) {
pod = &v1.Pod{
TypeMeta: metav1.TypeMeta{
- Kind: "Pod",
+ Kind: metadata.Pod,
APIVersion: "v1",
},
ObjectMeta: metav1.ObjectMeta{
@@ -171,7 +172,7 @@ func TestUpdateLabels(t *testing.T) {
pod = &v1.Pod{
TypeMeta: metav1.TypeMeta{
- Kind: "Pod",
+ Kind: metadata.Pod,
APIVersion: "v1",
},
ObjectMeta: metav1.ObjectMeta{
@@ -201,7 +202,7 @@ func TestUpdateLabels(t *testing.T) {
pod = &v1.Pod{
TypeMeta: metav1.TypeMeta{
- Kind: "Pod",
+ Kind: metadata.Pod,
APIVersion: "v1",
},
ObjectMeta: metav1.ObjectMeta{
@@ -229,7 +230,7 @@ func TestUpdateLabels(t *testing.T) {
pod = &v1.Pod{
TypeMeta: metav1.TypeMeta{
- Kind: "Pod",
+ Kind: metadata.Pod,
APIVersion: "v1",
},
ObjectMeta: metav1.ObjectMeta{},
@@ -389,6 +390,10 @@ func prepareConfigMap(data string) *v1.ConfigMap {
func prepareController(t *testing.T, url string, processNs string, bypassNs string, labelNs string, noLabelNs string, bypassAuth bool, trustControllers bool) *AdmissionController {
pcCache := createPriorityClassCacheForTest()
nsCache := createNamespaceClassCacheForTest()
+ nsCache.nameSpaces["disabled"] = nsFlags{
+ enableYuniKorn: FALSE,
+ generateAppID: FALSE,
+ }
if bypassNs == "" {
bypassNs = "^kube-system$"
}
@@ -460,7 +465,7 @@ func TestMutateYuniKorn(t *testing.T) {
req := &admissionv1.AdmissionRequest{
UID: "test-uid",
Namespace: schedulerconf.GetSchedulerNamespace(),
- Kind: metav1.GroupVersionKind{Kind: "Pod"},
+ Kind: metav1.GroupVersionKind{Kind: metadata.Pod},
}
podJSON, err := json.Marshal(pod)
assert.NilError(t, err, "failed to marshal pod")
@@ -486,7 +491,7 @@ func TestMutateYuniKorn(t *testing.T) {
req = &admissionv1.AdmissionRequest{
UID: "test-uid",
Namespace: "test-ns",
- Kind: metav1.GroupVersionKind{Kind: "Pod"},
+ Kind: metav1.GroupVersionKind{Kind: metadata.Pod},
}
podJSON, err = json.Marshal(pod)
assert.NilError(t, err, "failed to marshal pod")
@@ -508,7 +513,7 @@ func TestMutatePod(t *testing.T) {
req := &admissionv1.AdmissionRequest{
UID: "test-uid",
Namespace: "",
- Kind: metav1.GroupVersionKind{Kind: "Pod"},
+ Kind: metav1.GroupVersionKind{Kind: metadata.Pod},
}
podJSON, err := json.Marshal(pod)
assert.NilError(t, err, "failed to marshal pod")
@@ -526,7 +531,7 @@ func TestMutatePod(t *testing.T) {
req = &admissionv1.AdmissionRequest{
UID: "test-uid",
Namespace: "test-ns",
- Kind: metav1.GroupVersionKind{Kind: "Pod"},
+ Kind: metav1.GroupVersionKind{Kind: metadata.Pod},
}
podJSON, err = json.Marshal(pod)
assert.NilError(t, err, "failed to marshal pod")
@@ -553,7 +558,7 @@ func TestMutatePod(t *testing.T) {
req = &admissionv1.AdmissionRequest{
UID: "test-uid",
Namespace: "bypass",
- Kind: metav1.GroupVersionKind{Kind: "Pod"},
+ Kind: metav1.GroupVersionKind{Kind: metadata.Pod},
}
podJSON, err = json.Marshal(pod)
assert.NilError(t, err, "failed to marshal pod")
@@ -569,7 +574,7 @@ func TestMutatePod(t *testing.T) {
req = &admissionv1.AdmissionRequest{
UID: "test-uid",
Namespace: "nolabel",
- Kind: metav1.GroupVersionKind{Kind: "Pod"},
+ Kind: metav1.GroupVersionKind{Kind: metadata.Pod},
}
podJSON, err = json.Marshal(pod)
assert.NilError(t, err, "failed to marshal pod")
@@ -586,7 +591,7 @@ func TestMutatePod(t *testing.T) {
req = &admissionv1.AdmissionRequest{
UID: "test-uid",
Namespace: "test-ns",
- Kind: metav1.GroupVersionKind{Kind: "Pod"},
+ Kind: metav1.GroupVersionKind{Kind: metadata.Pod},
}
podJSON, err = json.Marshal(pod)
assert.NilError(t, err, "failed to marshal pod")
@@ -614,7 +619,7 @@ func TestMutateObject(t *testing.T) {
req := &admissionv1.AdmissionRequest{
UID: "test-uid",
Namespace: "test-ns",
- Kind: metav1.GroupVersionKind{Kind: "Deployment"},
+ Kind: metav1.GroupVersionKind{Kind: metadata.Deployment},
UserInfo: authv1.UserInfo{
Username: "testExtUser",
},
@@ -662,7 +667,7 @@ func TestMutateObject(t *testing.T) {
req = &admissionv1.AdmissionRequest{
UID: "test-uid",
Namespace: "test-ns",
- Kind: metav1.GroupVersionKind{Kind: "ReplicaSet"},
+ Kind: metav1.GroupVersionKind{Kind: metadata.ReplicaSet},
UserInfo: authv1.UserInfo{
Username: "system:serviceaccount:kube-system:deployment-controller",
Groups: []string{"system:serviceaccounts", "system:serviceaccounts:kube-system"},
@@ -691,7 +696,7 @@ func TestMutateUpdate(t *testing.T) {
req := &admissionv1.AdmissionRequest{
UID: "test-uid",
Namespace: "test-ns",
- Kind: metav1.GroupVersionKind{Kind: "Pod"},
+ Kind: metav1.GroupVersionKind{Kind: metadata.Pod},
Operation: admissionv1.Update,
}
podJSON, err := json.Marshal(pod)
@@ -708,7 +713,7 @@ func TestMutateUpdate(t *testing.T) {
req = &admissionv1.AdmissionRequest{
UID: "test-uid",
Namespace: "bypass",
- Kind: metav1.GroupVersionKind{Kind: "Pod"},
+ Kind: metav1.GroupVersionKind{Kind: metadata.Pod},
Operation: admissionv1.Update,
}
podJSON, err = json.Marshal(pod)
@@ -725,7 +730,7 @@ func TestMutateUpdate(t *testing.T) {
req = &admissionv1.AdmissionRequest{
UID: "test-uid",
Namespace: "test-ns",
- Kind: metav1.GroupVersionKind{Kind: "Pod"},
+ Kind: metav1.GroupVersionKind{Kind: metadata.Pod},
Operation: admissionv1.Update,
}
podJSON, err = json.Marshal(pod)
@@ -753,7 +758,7 @@ func TestMutateUpdate(t *testing.T) {
req = &admissionv1.AdmissionRequest{
UID: "test-uid",
Namespace: "test-ns",
- Kind: metav1.GroupVersionKind{Kind: "Pod"},
+ Kind: metav1.GroupVersionKind{Kind: metadata.Pod},
Operation: admissionv1.Update,
}
podJSON, err = json.Marshal(pod)
@@ -777,14 +782,14 @@ func TestMutateUser(t *testing.T) {
pod := v1.Pod{ObjectMeta: metav1.ObjectMeta{
Namespace: schedulerconf.GetSchedulerNamespace(),
Annotations: map[string]string{common.UserInfoAnnotation: validUserInfoAnnotation},
- }}
+ }, Spec: v1.PodSpec{SchedulerName: constants.SchedulerName}}
oldPod := v1.Pod{ObjectMeta: metav1.ObjectMeta{
Namespace: schedulerconf.GetSchedulerNamespace(),
- }}
+ }, Spec: v1.PodSpec{SchedulerName: constants.SchedulerName}}
req := &admissionv1.AdmissionRequest{
UID: "test-uid",
Namespace: schedulerconf.GetSchedulerNamespace(),
- Kind: metav1.GroupVersionKind{Kind: "Pod"},
+ Kind: metav1.GroupVersionKind{Kind: metadata.Pod},
Operation: admissionv1.Update,
}
// non yunikorn pod cannot change
@@ -825,11 +830,11 @@ func TestMutateAppID(t *testing.T) {
pod := v1.Pod{ObjectMeta: metav1.ObjectMeta{
Namespace: "test-ns",
Labels: map[string]string{constants.CanonicalLabelApplicationID: "app-1"},
- }}
+ }, Spec: v1.PodSpec{SchedulerName: constants.SchedulerName}}
req := &admissionv1.AdmissionRequest{
UID: "test-uid",
Namespace: "test-ns",
- Kind: metav1.GroupVersionKind{Kind: "Pod"},
+ Kind: metav1.GroupVersionKind{Kind: metadata.Pod},
Operation: admissionv1.Update,
}
podJSON, err := json.Marshal(pod)
@@ -846,11 +851,11 @@ func TestMutateAppID(t *testing.T) {
pod = v1.Pod{ObjectMeta: metav1.ObjectMeta{
Namespace: "test-ns",
Annotations: map[string]string{constants.AnnotationApplicationID: "app-1"},
- }}
+ }, Spec: v1.PodSpec{SchedulerName: constants.SchedulerName}}
req = &admissionv1.AdmissionRequest{
UID: "test-uid",
Namespace: "test-ns",
- Kind: metav1.GroupVersionKind{Kind: "Pod"},
+ Kind: metav1.GroupVersionKind{Kind: metadata.Pod},
Operation: admissionv1.Update,
}
podJSON, err = json.Marshal(pod)
@@ -862,6 +867,190 @@ func TestMutateAppID(t *testing.T) {
req.OldObject = runtime.RawExtension{Raw: podJSON}
resp = ac.mutate(req)
assert.Check(t, !resp.Allowed, "response allowed for appID mutation")
+
+ // app-1 pod adding canonical label overrides app annotation not scheduled by YuniKorn
+ pod = v1.Pod{ObjectMeta: metav1.ObjectMeta{
+ Namespace: "test-ns",
+ Annotations: map[string]string{constants.AnnotationApplicationID: "app-1"},
+ }, Spec: v1.PodSpec{SchedulerName: "default"}}
+ req = &admissionv1.AdmissionRequest{
+ UID: "test-uid",
+ Namespace: "test-ns",
+ Kind: metav1.GroupVersionKind{Kind: metadata.Pod},
+ Operation: admissionv1.Update,
+ }
+ podJSON, err = json.Marshal(pod)
+ assert.NilError(t, err, "failed to marshal pod")
+ req.Object = runtime.RawExtension{Raw: podJSON}
+ pod.Labels = map[string]string{constants.CanonicalLabelApplicationID: "app-2"}
+ podJSON, err = json.Marshal(pod)
+ assert.NilError(t, err, "failed to marshal pod")
+ req.OldObject = runtime.RawExtension{Raw: podJSON}
+ resp = ac.mutate(req)
+ assert.Check(t, resp.Allowed, "response not allowed for appID mutation not scheduled by yunikorn")
+
+ // appID change for yunikorn scheduled pod not allowed
+ pod = v1.Pod{ObjectMeta: metav1.ObjectMeta{
+ Namespace: "test-ns",
+ }, Spec: v1.PodSpec{SchedulerName: constants.SchedulerName}}
+ podJSON, err = json.Marshal(pod)
+ assert.NilError(t, err, "failed to marshal pod")
+ req.Object = runtime.RawExtension{Raw: podJSON}
+ pod.Labels = map[string]string{constants.CanonicalLabelApplicationID: "app-2"}
+ podJSON, err = json.Marshal(pod)
+ assert.NilError(t, err, "failed to marshal pod")
+ req.OldObject = runtime.RawExtension{Raw: podJSON}
+ resp = ac.mutate(req)
+ assert.Check(t, !resp.Allowed, "response allowed for appID mutation scheduled by yunikorn")
+}
+
+func TestMutateUpdateObjectUnknown(t *testing.T) {
+ ac := prepareController(t, "", "", "^kube-system$,^bypass$", "", "^nolabel$", true, true)
+
+ secret := &v1.Secret{
+ ObjectMeta: metav1.ObjectMeta{},
+ Data: map[string][]byte{"mysecret": {1, 2, 3}},
+ }
+ secretJSON, err := json.Marshal(secret)
+ assert.NilError(t, err, "failed to marshal secret")
+ req := &admissionv1.AdmissionRequest{
+ UID: "test-uid",
+ Namespace: "test-ns",
+ Kind: metav1.GroupVersionKind{Kind: "Secret"},
+ UserInfo: authv1.UserInfo{
+ Username: "testExtUser",
+ },
+ Operation: admissionv1.Update,
+ }
+ req.Object = runtime.RawExtension{Raw: secretJSON}
+ req.OldObject = runtime.RawExtension{Raw: secretJSON}
+ resp := ac.mutate(req)
+ assert.Check(t, resp.Allowed, "response not allowed for secret mutation")
+ job := &batchv1.Job{
+ Spec: batchv1.JobSpec{
+ Template: v1.PodTemplateSpec{
+ Spec: v1.PodSpec{
+ SchedulerName: constants.SchedulerName,
+ },
+ },
+ },
+ }
+ req = &admissionv1.AdmissionRequest{
+ UID: "test-uid",
+ Namespace: "test-ns",
+ Kind: metav1.GroupVersionKind{Kind: metadata.Job},
+ UserInfo: authv1.UserInfo{
+ Username: "testExtUser",
+ },
+ Operation: admissionv1.Update,
+ }
+ jobJSON, err := json.Marshal(job)
+ assert.NilError(t, err, "failed to marshal job")
+ req.Object = runtime.RawExtension{Raw: jobJSON}
+ req.OldObject = runtime.RawExtension{Raw: []byte{1, 2, 3}}
+ resp = ac.mutate(req)
+ assert.Check(t, !resp.Allowed, "response allowed for corrupt job mutation")
+
+ req = &admissionv1.AdmissionRequest{
+ UID: "test-uid",
+ Namespace: "disabled",
+ Kind: metav1.GroupVersionKind{Kind: metadata.CronJob},
+ UserInfo: authv1.UserInfo{
+ Username: "testExtUser",
+ },
+ Operation: admissionv1.Update,
+ }
+ cronJob := &batchv1.CronJob{
+ Spec: batchv1.CronJobSpec{
+ JobTemplate: batchv1.JobTemplateSpec{
+ Spec: batchv1.JobSpec{
+ Template: v1.PodTemplateSpec{
+ Spec: v1.PodSpec{
+ SchedulerName: "default",
+ },
+ },
+ },
+ },
+ },
+ }
+ cronJobJSON, err := json.Marshal(cronJob)
+ assert.NilError(t, err, "failed to marshal cronjob")
+ req.Object = runtime.RawExtension{Raw: cronJobJSON}
+ cronJob.Spec.JobTemplate.Spec.Template.Annotations = map[string]string{common.UserInfoAnnotation: "testExtUser"}
+ req.OldObject = runtime.RawExtension{Raw: cronJobJSON}
+ resp = ac.mutate(req)
+ assert.Check(t, resp.Allowed, "response allowed for non yunikorn namespace")
+}
+
+func TestMutateUpdateObject(t *testing.T) {
+ ac := prepareController(t, "", "", "^kube-system$,^bypass$", "", "^nolabel$", true, true)
+
+ // nil request
+ resp := ac.mutate(nil)
+ assert.Check(t, !resp.Allowed, "response allowed with nil request")
+
+ deployment := &appsv1.Deployment{
+ Spec: appsv1.DeploymentSpec{
+ Template: v1.PodTemplateSpec{
+ ObjectMeta: metav1.ObjectMeta{
+ Annotations: map[string]string{
+ common.UserInfoAnnotation: validUserInfoAnnotation,
+ },
+ },
+ },
+ },
+ }
+ req := &admissionv1.AdmissionRequest{
+ UID: "test-uid",
+ Namespace: "test-ns",
+ Kind: metav1.GroupVersionKind{Kind: metadata.Deployment},
+ UserInfo: authv1.UserInfo{
+ Username: "testExtUser",
+ },
+ Operation: admissionv1.Update,
+ }
+ deploymentJSON, err := json.Marshal(deployment)
+ assert.NilError(t, err, "failed to marshal deployment")
+ req.Object = runtime.RawExtension{Raw: deploymentJSON}
+ req.OldObject = runtime.RawExtension{Raw: deploymentJSON}
+ resp = ac.mutate(req)
+ assert.Check(t, resp.Allowed, "response not allowed for deployment")
+
+ // no user in old deployment
+ deployment.Spec.Template.Annotations = map[string]string{}
+ deploymentJSON, err = json.Marshal(deployment)
+ assert.NilError(t, err, "failed to marshal deployment")
+ req.OldObject = runtime.RawExtension{Raw: deploymentJSON}
+ resp = ac.mutate(req)
+ assert.Check(t, !resp.Allowed, "response allowed for deployment user change")
+
+ // appID in old deployment
+ deployment.Spec.Template.Annotations = map[string]string{common.UserInfoAnnotation: validUserInfoAnnotation}
+ deployment.Spec.Template.Labels = map[string]string{constants.CanonicalLabelApplicationID: "app-1"}
+ deploymentJSON, err = json.Marshal(deployment)
+ assert.NilError(t, err, "failed to marshal deployment")
+ req.OldObject = runtime.RawExtension{Raw: deploymentJSON}
+ resp = ac.mutate(req)
+ assert.Check(t, !resp.Allowed, "response allowed for deployment appID change")
+
+ // update replicaset
+ req = &admissionv1.AdmissionRequest{
+ UID: "test-uid",
+ Namespace: "test-ns",
+ Kind: metav1.GroupVersionKind{Kind: metadata.ReplicaSet},
+ UserInfo: authv1.UserInfo{
+ Username: "system:serviceaccount:kube-system:deployment-controller",
+ Groups: []string{"system:serviceaccounts", "system:serviceaccounts:kube-system"},
+ },
+ Operation: admissionv1.Update,
+ }
+ replSet := &appsv1.ReplicaSet{}
+ replSetJSON, err := json.Marshal(replSet)
+ assert.NilError(t, err, "failed to marshal replSet")
+ req.OldObject = runtime.RawExtension{Raw: replSetJSON}
+ req.Object = runtime.RawExtension{Raw: replSetJSON}
+ resp = ac.mutate(req)
+ assert.Check(t, resp.Allowed, "response allowed for replSet change")
}
func TestExternalAuthentication(t *testing.T) {
@@ -879,14 +1068,14 @@ func TestExternalAuthentication(t *testing.T) {
req := &admissionv1.AdmissionRequest{
UID: "test-uid",
Namespace: "test-ns",
- Kind: metav1.GroupVersionKind{Kind: "Pod"},
+ Kind: metav1.GroupVersionKind{Kind: metadata.Pod},
UserInfo: authv1.UserInfo{
Username: "test",
Groups: []string{"dev"},
},
}
req.Object = runtime.RawExtension{Raw: podJSON}
- req.Kind = metav1.GroupVersionKind{Kind: "Pod"}
+ req.Kind = metav1.GroupVersionKind{Kind: metadata.Pod}
resp := ac.mutate(req)
assert.Check(t, !resp.Allowed, "response was allowed")
assert.Check(t, strings.Contains(resp.Result.Message, "not allowed to set user annotation"))
@@ -895,14 +1084,14 @@ func TestExternalAuthentication(t *testing.T) {
req = &admissionv1.AdmissionRequest{
UID: "test-uid",
Namespace: "test-ns",
- Kind: metav1.GroupVersionKind{Kind: "Pod"},
+ Kind: metav1.GroupVersionKind{Kind: metadata.Pod},
UserInfo: authv1.UserInfo{
Username: "testExtUser",
Groups: []string{"dev"},
},
}
req.Object = runtime.RawExtension{Raw: podJSON}
- req.Kind = metav1.GroupVersionKind{Kind: "Pod"}
+ req.Kind = metav1.GroupVersionKind{Kind: metadata.Pod}
resp = ac.mutate(req)
assert.Check(t, resp.Allowed, "response not allowed")
@@ -916,7 +1105,7 @@ func TestExternalAuthentication(t *testing.T) {
podJSON, err = json.Marshal(pod)
assert.NilError(t, err, "failed to marshal pod")
req.Object = runtime.RawExtension{Raw: podJSON}
- req.Kind = metav1.GroupVersionKind{Kind: "Pod"}
+ req.Kind = metav1.GroupVersionKind{Kind: metadata.Pod}
resp = ac.mutate(req)
assert.Check(t, !resp.Allowed, "response was allowed")
assert.Check(t, strings.Contains(resp.Result.Message, "invalid character 'x'"))
@@ -937,7 +1126,7 @@ func TestExternalAuthentication(t *testing.T) {
deploymentJSON, err2 := json.Marshal(deployment)
assert.NilError(t, err2, "failed to marshal deployment")
req.Object = runtime.RawExtension{Raw: deploymentJSON}
- req.Kind = metav1.GroupVersionKind{Kind: "Deployment"}
+ req.Kind = metav1.GroupVersionKind{Kind: metadata.Deployment}
resp = ac.mutate(req)
assert.Check(t, resp.Allowed, "response not allowed")
@@ -945,7 +1134,7 @@ func TestExternalAuthentication(t *testing.T) {
deployment.Spec.Template.Annotations[common.UserInfoAnnotation] = "xyzxyz"
deploymentJSON, err = json.Marshal(deployment)
req.Object = runtime.RawExtension{Raw: deploymentJSON}
- req.Kind = metav1.GroupVersionKind{Kind: "Deployment"}
+ req.Kind = metav1.GroupVersionKind{Kind: metadata.Deployment}
assert.NilError(t, err, "failed to marshal deployment")
resp = ac.mutate(req)
assert.Check(t, !resp.Allowed, "response was allowed")
@@ -967,7 +1156,7 @@ func TestExternalAuthentication(t *testing.T) {
replicaSetJSON, err3 := json.Marshal(replicaSet)
assert.NilError(t, err3, "failed to marshal replicaset")
req.Object = runtime.RawExtension{Raw: replicaSetJSON}
- req.Kind = metav1.GroupVersionKind{Kind: "ReplicaSet"}
+ req.Kind = metav1.GroupVersionKind{Kind: metadata.ReplicaSet}
req.UserInfo = authv1.UserInfo{
Username: "system:serviceaccount:kube-system:deployment-controller",
Groups: []string{"system:serviceaccounts", "system:serviceaccounts:kube-system"},
diff --git a/pkg/admission/common/common.go b/pkg/admission/common/common.go
index 78c83989..36d7b697 100644
--- a/pkg/admission/common/common.go
+++ b/pkg/admission/common/common.go
@@ -18,7 +18,13 @@
package common
-import siCommon "github.com/apache/yunikorn-scheduler-interface/lib/go/common"
+import (
+ "errors"
+
+ siCommon "github.com/apache/yunikorn-scheduler-interface/lib/go/common"
+)
+
+var ErrorUnsupportedKind = errors.New("unsupported object kind")
const (
UserInfoAnnotation = siCommon.DomainYuniKorn + "user.info"
diff --git a/pkg/admission/metadata/extract.go b/pkg/admission/metadata/extract.go
index 1006981e..d70ce73e 100644
--- a/pkg/admission/metadata/extract.go
+++ b/pkg/admission/metadata/extract.go
@@ -25,8 +25,10 @@ import (
admissionv1 "k8s.io/api/admission/v1"
appsv1 "k8s.io/api/apps/v1"
batchv1 "k8s.io/api/batch/v1"
- batchv1Beta "k8s.io/api/batch/v1beta1"
corev1 "k8s.io/api/core/v1"
+ "k8s.io/apimachinery/pkg/runtime"
+
+ "github.com/apache/yunikorn-k8shim/pkg/admission/common"
)
type extractResult struct {
@@ -35,30 +37,48 @@ type extractResult struct {
path string
}
-type extractor func(*admissionv1.AdmissionRequest) (*extractResult, error)
+type extractorFn func(obj runtime.RawExtension) (*extractResult, error)
var (
- Deployment = reflect.TypeOf(appsv1.Deployment{}).Name()
- DaemonSet = reflect.TypeOf(appsv1.DaemonSet{}).Name()
- StatefulSet = reflect.TypeOf(appsv1.StatefulSet{}).Name()
- ReplicaSet = reflect.TypeOf(appsv1.ReplicaSet{}).Name()
- Job = reflect.TypeOf(batchv1.Job{}).Name()
- CronJob = reflect.TypeOf(batchv1Beta.CronJob{}).Name()
- Pod = reflect.TypeOf(corev1.Pod{}).Name()
-
- extractors = map[string]extractor{
+ Deployment = reflect.TypeFor[appsv1.Deployment]().Name()
+ DaemonSet = reflect.TypeFor[appsv1.DaemonSet]().Name()
+ StatefulSet = reflect.TypeFor[appsv1.StatefulSet]().Name()
+ ReplicaSet = reflect.TypeFor[appsv1.ReplicaSet]().Name()
+ Job = reflect.TypeFor[batchv1.Job]().Name()
+ CronJob = reflect.TypeFor[batchv1.CronJob]().Name()
+ Pod = reflect.TypeFor[corev1.Pod]().Name()
+
+ extractors = map[string]extractorFn{
Deployment: fromDeployment,
DaemonSet: fromDaemonSet,
StatefulSet: fromStatefulSet,
ReplicaSet: fromReplicaSet,
Job: fromJob,
CronJob: fromCronJob,
+ Pod: fromPod,
}
)
-func fromDeployment(req *admissionv1.AdmissionRequest) (*extractResult, error) {
+func getResultFromRequest(req *admissionv1.AdmissionRequest) (*extractResult, error) {
+ return extractFromReq(req, false)
+}
+
+// extractFromReq loads the labels and annotations from the object, can handle create and update requests.
+// Supports all kinds.
+func extractFromReq(req *admissionv1.AdmissionRequest, old bool) (*extractResult, error) {
+ extractFn, ok := extractors[req.Kind.Kind]
+ if !ok {
+ return nil, common.ErrorUnsupportedKind
+ }
+ if old {
+ return extractFn(req.OldObject)
+ }
+ return extractFn(req.Object)
+}
+
+func fromDeployment(obj runtime.RawExtension) (*extractResult, error) {
var deployment appsv1.Deployment
- err := json.Unmarshal(req.Object.Raw, &deployment)
+ err := json.Unmarshal(obj.Raw, &deployment)
if err != nil {
return nil, err
}
@@ -70,9 +90,9 @@ func fromDeployment(req *admissionv1.AdmissionRequest) (*extractResult, error) {
}, nil
}
-func fromDaemonSet(req *admissionv1.AdmissionRequest) (*extractResult, error) {
+func fromDaemonSet(obj runtime.RawExtension) (*extractResult, error) {
var daemonSet appsv1.DaemonSet
- err := json.Unmarshal(req.Object.Raw, &daemonSet)
+ err := json.Unmarshal(obj.Raw, &daemonSet)
if err != nil {
return nil, err
}
@@ -84,9 +104,9 @@ func fromDaemonSet(req *admissionv1.AdmissionRequest) (*extractResult, error) {
}, nil
}
-func fromStatefulSet(req *admissionv1.AdmissionRequest) (*extractResult, error) {
+func fromStatefulSet(obj runtime.RawExtension) (*extractResult, error) {
var statefulSet appsv1.StatefulSet
- err := json.Unmarshal(req.Object.Raw, &statefulSet)
+ err := json.Unmarshal(obj.Raw, &statefulSet)
if err != nil {
return nil, err
}
@@ -98,9 +118,9 @@ func fromStatefulSet(req *admissionv1.AdmissionRequest) (*extractResult, error)
}, nil
}
-func fromReplicaSet(req *admissionv1.AdmissionRequest) (*extractResult, error) {
+func fromReplicaSet(obj runtime.RawExtension) (*extractResult, error) {
var replicaSet appsv1.ReplicaSet
- err := json.Unmarshal(req.Object.Raw, &replicaSet)
+ err := json.Unmarshal(obj.Raw, &replicaSet)
if err != nil {
return nil, err
}
@@ -112,9 +132,9 @@ func fromReplicaSet(req *admissionv1.AdmissionRequest) (*extractResult, error) {
}, nil
}
-func fromJob(req *admissionv1.AdmissionRequest) (*extractResult, error) {
+func fromJob(obj runtime.RawExtension) (*extractResult, error) {
var job batchv1.Job
- err := json.Unmarshal(req.Object.Raw, &job)
+ err := json.Unmarshal(obj.Raw, &job)
if err != nil {
return nil, err
}
@@ -126,9 +146,9 @@ func fromJob(req *admissionv1.AdmissionRequest) (*extractResult, error) {
}, nil
}
-func fromCronJob(req *admissionv1.AdmissionRequest) (*extractResult, error) {
- var cronJob batchv1Beta.CronJob
- err := json.Unmarshal(req.Object.Raw, &cronJob)
+func fromCronJob(obj runtime.RawExtension) (*extractResult, error) {
+ var cronJob batchv1.CronJob
+ err := json.Unmarshal(obj.Raw, &cronJob)
if err != nil {
return nil, err
}
@@ -139,3 +159,17 @@ func fromCronJob(req *admissionv1.AdmissionRequest) (*extractResult, error) {
path: cronJobPodAnnotationsPath,
}, nil
}
+
+func fromPod(obj runtime.RawExtension) (*extractResult, error) {
+ var pod corev1.Pod
+ err := json.Unmarshal(obj.Raw, &pod)
+ if err != nil {
+ return nil, err
+ }
+
+ return &extractResult{
+ annotations: pod.Annotations,
+ labels: pod.Labels,
+ path: PodAnnotationsPath,
+ }, nil
+}
diff --git a/pkg/admission/metadata/labels.go b/pkg/admission/metadata/labels.go
index 808c5a04..35cd6954 100644
--- a/pkg/admission/metadata/labels.go
+++ b/pkg/admission/metadata/labels.go
@@ -18,17 +18,23 @@
package metadata
-import admissionv1 "k8s.io/api/admission/v1"
+import (
+ "errors"
+
+ admissionv1 "k8s.io/api/admission/v1"
+
+ "github.com/apache/yunikorn-k8shim/pkg/admission/common"
+)
type LabelExtractor struct{}
-func (l *LabelExtractor) GetLabelsFromWorkload(req *admissionv1.AdmissionRequest) (map[string]string, bool, error) {
- extractFn, ok := extractors[req.Kind.Kind]
- if !ok {
+// GetLabelsFromRequest loads the labels from the workload object, can handle create and update requests.
+func (l *LabelExtractor) GetLabelsFromRequest(req *admissionv1.AdmissionRequest, old bool) (map[string]string, bool, error) {
+ result, err := extractFromReq(req, old)
+ if errors.Is(err, common.ErrorUnsupportedKind) {
return nil, false, nil
}
- result, err := extractFn(req)
- if result == nil {
+ if err != nil {
return nil, true, err
}
return result.labels, true, err
diff --git a/pkg/admission/metadata/labels_test.go b/pkg/admission/metadata/labels_test.go
index f52010c2..dc52435b 100644
--- a/pkg/admission/metadata/labels_test.go
+++ b/pkg/admission/metadata/labels_test.go
@@ -34,7 +34,7 @@ func TestGetLabelFromWorkload(t *testing.T) {
for _, testCase := range tests {
t.Run("TestGetLabelsFromWorkload#"+testCase.kind, func(t *testing.T) {
req := getAdmissionRequest(t, testCase.obj, testCase.kind)
- labels, supported, err := le.GetLabelsFromWorkload(req)
+ labels, supported, err := le.GetLabelsFromRequest(req, false)
assert.Assert(t, supported)
assert.NilError(t, err)
assert.Assert(t, labels != nil)
@@ -51,7 +51,7 @@ func TestGetLabelFromWorkloadFails(t *testing.T) {
t.Run("TestGetLabelFromWorkloadFails#"+testCase.kind, func(t *testing.T) {
req := getAdmissionRequest(t, nil, testCase.kind)
req.Object.Raw = []byte{0, 1, 2, 3, 4}
- labels, supported, err := le.GetLabelsFromWorkload(req)
+ labels, supported, err := le.GetLabelsFromRequest(req, false)
assert.Assert(t, supported)
assert.ErrorContains(t, err, "invalid character")
assert.Assert(t, labels == nil)
@@ -66,7 +66,7 @@ func TestGetLabelFromUnknownObject(t *testing.T) {
Kind: "Unknown",
},
}
- labels, supported, err := le.GetLabelsFromWorkload(req)
+ labels, supported, err := le.GetLabelsFromRequest(req, false)
assert.Check(t, labels == nil)
assert.Check(t, !supported)
assert.NilError(t, err)
@@ -76,7 +76,7 @@ func TestGetLabelFromInvalidObject(t *testing.T) {
req := getAdmissionRequest(t, nil, "Deployment")
req.Object.Raw = []byte{0, 1, 2, 3, 4}
le := &LabelExtractor{}
- labels, supported, err := le.GetLabelsFromWorkload(req)
+ labels, supported, err := le.GetLabelsFromRequest(req, false)
assert.Check(t, labels == nil)
assert.Check(t, supported)
assert.ErrorContains(t, err, "invalid character")
diff --git a/pkg/admission/metadata/usergroup.go b/pkg/admission/metadata/usergroup.go
index a93ad304..ace3a207 100644
--- a/pkg/admission/metadata/usergroup.go
+++ b/pkg/admission/metadata/usergroup.go
@@ -20,6 +20,7 @@ package metadata
import (
"encoding/json"
+ "errors"
"strings"
"go.uber.org/zap"
@@ -45,6 +46,7 @@ func NewUserGroupAnnotationHandler(conf *conf.AdmissionControllerConf) *UserGrou
const (
defaultPodAnnotationsPath = "/spec/template/metadata/annotations"
cronJobPodAnnotationsPath = "/spec/jobTemplate/spec/template/metadata/annotations"
+ PodAnnotationsPath = "/metadata/annotations"
)
func (u *UserGroupAnnotationHandler) IsAnnotationAllowed(userName string, groups []string) bool {
@@ -93,24 +95,19 @@ func (u *UserGroupAnnotationHandler) IsAnnotationValid(userInfoAnnotation string
return nil
}
-func (u *UserGroupAnnotationHandler) GetAnnotationsFromRequestKind(req *admissionv1.AdmissionRequest) (map[string]string, bool, error) {
- extractFn, ok := extractors[req.Kind.Kind]
- if !ok {
+func (u *UserGroupAnnotationHandler) GetAnnotationsFromRequest(req *admissionv1.AdmissionRequest, old bool) (map[string]string, bool, error) {
+ result, err := extractFromReq(req, old)
+ if errors.Is(err, common.ErrorUnsupportedKind) {
return nil, false, nil
}
- result, err := extractFn(req)
- if result == nil {
+ if err != nil {
return nil, true, err
}
return result.annotations, true, err
}
-func (u *UserGroupAnnotationHandler) GetPatchForWorkload(req *admissionv1.AdmissionRequest, user string, groups []string) ([]common.PatchOperation, error) {
- extractFn, ok := extractors[req.Kind.Kind]
- if !ok {
- return nil, nil
- }
- result, err := extractFn(req)
+func (u *UserGroupAnnotationHandler) GetPatchForWorkload(req *admissionv1.AdmissionRequest, user string, groups []string) (*common.PatchOperation, error) {
+ result, err := getResultFromRequest(req)
if err != nil {
return nil, err
}
@@ -119,15 +116,11 @@ func (u *UserGroupAnnotationHandler) GetPatchForWorkload(req *admissionv1.Admiss
if patchErr != nil {
return nil, patchErr
}
-
- patch := make([]common.PatchOperation, 1)
- patch[0] = *patchOp
-
- return patch, nil
+ return patchOp, nil
}
func (u *UserGroupAnnotationHandler) GetPatchForPod(annotations map[string]string, user string, groups []string) (*common.PatchOperation, error) {
- patchOp, err := u.getPatchOperation(annotations, "/metadata/annotations", user, groups)
+ patchOp, err := u.getPatchOperation(annotations, PodAnnotationsPath, user, groups)
if err != nil {
return nil, err
}
diff --git a/pkg/admission/metadata/usergroup_test.go b/pkg/admission/metadata/usergroup_test.go
index 5dc4a912..ca20ed99 100644
--- a/pkg/admission/metadata/usergroup_test.go
+++ b/pkg/admission/metadata/usergroup_test.go
@@ -27,7 +27,6 @@ import (
admissionv1 "k8s.io/api/admission/v1"
appsv1 "k8s.io/api/apps/v1"
batchv1 "k8s.io/api/batch/v1"
- batchv1Beta "k8s.io/api/batch/v1beta1"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
@@ -104,7 +103,7 @@ func TestGetAnnotationFromRequest(t *testing.T) {
for _, testCase := range tests {
t.Run("TestGetAnnotationFromRequest#"+testCase.kind, func(t *testing.T) {
req := getAdmissionRequest(t, testCase.obj, testCase.kind)
- annotations, supported, err := ah.GetAnnotationsFromRequestKind(req)
+ annotations, supported, err := ah.GetAnnotationsFromRequest(req, false)
assert.Assert(t, supported)
assert.NilError(t, err)
assert.Assert(t, annotations != nil)
@@ -121,7 +120,7 @@ func TestGetAnnotationFromRequestFails(t *testing.T) {
t.Run("TestGetAnnotationFromRequestFails#"+testCase.kind, func(t *testing.T) {
req := getAdmissionRequest(t, nil, testCase.kind)
req.Object.Raw = []byte{0, 1, 2, 3, 4}
- annotations, supported, err := ah.GetAnnotationsFromRequestKind(req)
+ annotations, supported, err := ah.GetAnnotationsFromRequest(req, false)
assert.Assert(t, supported)
assert.ErrorContains(t, err, "invalid character")
assert.Assert(t, annotations == nil)
@@ -136,7 +135,7 @@ func TestGetAnnotationFromUnknownObject(t *testing.T) {
Kind: "Unknown",
},
}
- annotations, supported, err := ah.GetAnnotationsFromRequestKind(req)
+ annotations, supported, err := ah.GetAnnotationsFromRequest(req, false)
assert.Check(t, annotations == nil)
assert.Check(t, !supported)
assert.NilError(t, err)
@@ -146,7 +145,7 @@ func TestGetAnnotationFromInvalidObject(t *testing.T) {
req := getAdmissionRequest(t, nil, "Deployment")
req.Object.Raw = []byte{0, 1, 2, 3, 4}
ah := getAnnotationHandler()
- annotations, supported, err := ah.GetAnnotationsFromRequestKind(req)
+ annotations, supported, err := ah.GetAnnotationsFromRequest(req, false)
assert.Check(t, annotations == nil)
assert.Check(t, supported)
assert.ErrorContains(t, err, "invalid character")
@@ -159,10 +158,8 @@ func TestGetPatchForWorkload(t *testing.T) {
for _, testCase := range tests {
t.Run("TestGetPatchForWorkload#"+testCase.kind, func(t *testing.T) {
req := getAdmissionRequest(t, testCase.obj, testCase.kind)
- patch, err := ah.GetPatchForWorkload(req, "yunikorn", []string{"users", "dev"})
+ patchOp, err := ah.GetPatchForWorkload(req, "yunikorn", []string{"users", "dev"})
assert.NilError(t, err)
- assert.Equal(t, 1, len(patch))
- patchOp := patch[0]
assert.Equal(t, patchOp.Op, "add")
assert.Equal(t, patchOp.Path, testCase.path)
verifyUserGroupAnnotation(t, patchOp.Value)
@@ -174,9 +171,8 @@ func TestGetPatchForPod(t *testing.T) {
ah := getAnnotationHandler()
patchOp, err := ah.GetPatchForPod(annotation, "yunikorn", []string{"users", "dev"})
assert.NilError(t, err)
- assert.Assert(t, patchOp != nil)
assert.Equal(t, patchOp.Op, "add")
- assert.Equal(t, patchOp.Path, "/metadata/annotations")
+ assert.Equal(t, patchOp.Path, PodAnnotationsPath)
verifyUserGroupAnnotation(t, patchOp.Value)
}
@@ -269,9 +265,9 @@ func getTestCases() []TestCase {
path: defaultPodAnnotationsPath,
},
{
- obj: &batchv1Beta.CronJob{
- Spec: batchv1Beta.CronJobSpec{
- JobTemplate: batchv1Beta.JobTemplateSpec{
+ obj: &batchv1.CronJob{
+ Spec: batchv1.CronJobSpec{
+ JobTemplate: batchv1.JobTemplateSpec{
Spec: batchv1.JobSpec{
Template: v1.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
diff --git a/pkg/admission/util.go b/pkg/admission/util.go
index 5fe86486..86c5ce86 100644
--- a/pkg/admission/util.go
+++ b/pkg/admission/util.go
@@ -84,8 +84,34 @@ func convert2Namespace(obj interface{}) *v1.Namespace {
// changedAppID returns true if the application ID has changed in an update
func changedAppID(oldPod, newPod *v1.Pod) bool {
- oldID := utils.GetApplicationIDValue(oldPod)
- newID := utils.GetApplicationIDValue(newPod)
+ oldID := getApplicationIDValue(oldPod.Labels, oldPod.Annotations)
+ newID := getApplicationIDValue(newPod.Labels, newPod.Annotations)
return oldID != newID
}
+
+// getApplicationIDValue retrieves the application ID from the pod based on the predefined order
+// Application ID can be defined in multiple places
+// The application ID is determined by the following order.
+// 1. Label: constants.CanonicalLabelApplicationID
+// 2. Annotation: constants.AnnotationApplicationID
+// 3. Label: constants.LabelApplicationID
+// 4. Label: constants.SparkLabelAppID
+func getApplicationIDValue(labels, annotations map[string]string) string {
+ appID := labels[constants.CanonicalLabelApplicationID]
+
+ if appID == "" {
+ appID = annotations[constants.AnnotationApplicationID]
+ }
+ if appID == "" {
+ appID = labels[constants.LabelApplicationID]
+ }
+ if appID == "" {
+ appID = labels[constants.SparkLabelAppID]
+ }
+ return appID
+}
+
+func isScheduledByYuniKorn(pod *v1.Pod) bool {
+ return pod.Spec.SchedulerName == constants.SchedulerName
+}
diff --git a/pkg/cache/task.go b/pkg/cache/task.go
index 20da78b6..5c54a5df 100644
--- a/pkg/cache/task.go
+++ b/pkg/cache/task.go
@@ -585,7 +585,7 @@ func (task *Task) checkPodMetadataBeforeScheduling() {
}
}
-func (task *Task) logIgnoredPodMetadata(pod *v1.Pod, metadataType string, fianlValue string, ignoredLabel map[string]string, ignoredAnnotation map[string]string) {
+func (task *Task) logIgnoredPodMetadata(pod *v1.Pod, metadataType string, finalValue string, ignoredLabel map[string]string, ignoredAnnotation map[string]string) {
ignoredItems := make([]string, 0)
for key, value := range ignoredLabel {
ignoredItems = append(ignoredItems, fmt.Sprintf("(Label) %s: %s", key, value))
@@ -593,8 +593,8 @@ func (task *Task) logIgnoredPodMetadata(pod *v1.Pod, metadataType string, fianlV
for key, value := range ignoredAnnotation {
ignoredItems = append(ignoredItems, fmt.Sprintf("(Annotation) %s: %s", key, value))
}
- logMessage := fmt.Sprintf("Found multiple '%s' value in pod. { podName: %s, fianlValue: %s, ignored: [%s] }",
- metadataType, pod.Name, fianlValue, strings.Join(ignoredItems, ", "))
+ logMessage := fmt.Sprintf("Found multiple '%s' value in pod. { podName: %s, finalValue: %s, ignored: [%s] }",
+ metadataType, pod.Name, finalValue, strings.Join(ignoredItems, ", "))
log.Log(log.ShimCacheTask).Warn(logMessage)
events.GetRecorder().Eventf(pod.DeepCopy(),
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@yunikorn.apache.org
For additional commands, e-mail: issues-help@yunikorn.apache.org