Skip to content

Commit

Permalink
Implementation of KEP-3589: uniformly filter manageJobsWithoutQueueNa…
Browse files Browse the repository at this point in the history
…mes by namespace (#3712)

* core implementation of managedJobsNamespaceSelector

* add commented out example managedJobsNamespaceSelector to config

* validation of managedJobsNamespaceSelector

* defaulting of managedJobsNamespaceSelector

* update generated files

* generate-apiref

* kep status is implementable

* add ManagedJobsNamespaceSelector to featuregate table

* linter fix: context.Context should be first argument

* address comments from first round of reviews

1. parse label selector during startup and used cached result
2. guard all uses with feature gate

* fixes to get unit tests to pass

* linter fix

* updating integration tests to pass new jobframework option

* Update pkg/controller/jobframework/defaults.go

Co-authored-by: Yuki Iwai <yuki.iwai.tz@gmail.com>

* integration tests for managedJobNamespaceSelector functionality

* linter fix

---------

Co-authored-by: Yuki Iwai <yuki.iwai.tz@gmail.com>
  • Loading branch information
dgrove-oss and tenzen-y authored Dec 5, 2024
1 parent eae2b14 commit 17e0fe8
Show file tree
Hide file tree
Showing 48 changed files with 987 additions and 292 deletions.
3 changes: 3 additions & 0 deletions apis/config/v1beta1/configuration_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ type Configuration struct {
// unsuspended, they will start immediately.
ManageJobsWithoutQueueName bool `json:"manageJobsWithoutQueueName"`

// ManagedJobsNamespaceSelector can be used to omit some namespaces from ManagedJobsWithoutQueueName
ManagedJobsNamespaceSelector *metav1.LabelSelector `json:"managedJobsNamespaceSelector,omitempty"`

// InternalCertManagement is configuration for internalCertManagement
InternalCertManagement *InternalCertManagement `json:"internalCertManagement,omitempty"`

Expand Down
14 changes: 14 additions & 0 deletions apis/config/v1beta1/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,20 @@ func SetDefaults_Configuration(cfg *Configuration) {
cfg.Integrations.PodOptions.PodSelector = &metav1.LabelSelector{}
}

if cfg.ManagedJobsNamespaceSelector == nil {
matchExpressionsValues := []string{"kube-system", *cfg.Namespace}

cfg.ManagedJobsNamespaceSelector = &metav1.LabelSelector{
MatchExpressions: []metav1.LabelSelectorRequirement{
{
Key: "kubernetes.io/metadata.name",
Operator: metav1.LabelSelectorOpNotIn,
Values: matchExpressionsValues,
},
},
}
}

if cfg.MultiKueue == nil {
cfg.MultiKueue = &MultiKueue{}
}
Expand Down
144 changes: 90 additions & 54 deletions apis/config/v1beta1/defaults_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,15 @@ func TestSetDefaults_Configuration(t *testing.T) {
MaxCount: 10,
},
}
defaultManagedJobsNamespaceSelector := &metav1.LabelSelector{
MatchExpressions: []metav1.LabelSelectorRequirement{
{
Key: "kubernetes.io/metadata.name",
Operator: metav1.LabelSelectorOpNotIn,
Values: []string{"kube-system", "kueue-system"},
},
},
}

overwriteNamespaceIntegrations := &Integrations{
Frameworks: []string{defaultJobFrameworkName},
Expand All @@ -97,6 +106,16 @@ func TestSetDefaults_Configuration(t *testing.T) {
},
}

overwriteNamespaceSelector := &metav1.LabelSelector{
MatchExpressions: []metav1.LabelSelectorRequirement{
{
Key: "kubernetes.io/metadata.name",
Operator: metav1.LabelSelectorOpNotIn,
Values: []string{"kube-system", overwriteNamespace},
},
},
}

defaultMultiKueue := &MultiKueue{
GCInterval: &metav1.Duration{Duration: DefaultMultiKueueGCInterval},
Origin: ptr.To(DefaultMultiKueueOrigin),
Expand All @@ -122,10 +141,11 @@ func TestSetDefaults_Configuration(t *testing.T) {
InternalCertManagement: &InternalCertManagement{
Enable: ptr.To(false),
},
ClientConnection: defaultClientConnection,
Integrations: defaultIntegrations,
QueueVisibility: defaultQueueVisibility,
MultiKueue: defaultMultiKueue,
ClientConnection: defaultClientConnection,
Integrations: defaultIntegrations,
QueueVisibility: defaultQueueVisibility,
MultiKueue: defaultMultiKueue,
ManagedJobsNamespaceSelector: defaultManagedJobsNamespaceSelector,
},
},
"defaulting ControllerManager": {
Expand Down Expand Up @@ -163,10 +183,11 @@ func TestSetDefaults_Configuration(t *testing.T) {
InternalCertManagement: &InternalCertManagement{
Enable: ptr.To(false),
},
ClientConnection: defaultClientConnection,
Integrations: defaultIntegrations,
QueueVisibility: defaultQueueVisibility,
MultiKueue: defaultMultiKueue,
ClientConnection: defaultClientConnection,
Integrations: defaultIntegrations,
QueueVisibility: defaultQueueVisibility,
MultiKueue: defaultMultiKueue,
ManagedJobsNamespaceSelector: defaultManagedJobsNamespaceSelector,
},
},
"should not default ControllerManager": {
Expand Down Expand Up @@ -220,10 +241,11 @@ func TestSetDefaults_Configuration(t *testing.T) {
InternalCertManagement: &InternalCertManagement{
Enable: ptr.To(false),
},
ClientConnection: defaultClientConnection,
Integrations: defaultIntegrations,
QueueVisibility: defaultQueueVisibility,
MultiKueue: defaultMultiKueue,
ClientConnection: defaultClientConnection,
Integrations: defaultIntegrations,
QueueVisibility: defaultQueueVisibility,
MultiKueue: defaultMultiKueue,
ManagedJobsNamespaceSelector: defaultManagedJobsNamespaceSelector,
},
},
"should not set LeaderElectionID": {
Expand Down Expand Up @@ -261,10 +283,11 @@ func TestSetDefaults_Configuration(t *testing.T) {
InternalCertManagement: &InternalCertManagement{
Enable: ptr.To(false),
},
ClientConnection: defaultClientConnection,
Integrations: defaultIntegrations,
QueueVisibility: defaultQueueVisibility,
MultiKueue: defaultMultiKueue,
ClientConnection: defaultClientConnection,
Integrations: defaultIntegrations,
QueueVisibility: defaultQueueVisibility,
MultiKueue: defaultMultiKueue,
ManagedJobsNamespaceSelector: defaultManagedJobsNamespaceSelector,
},
},
"defaulting InternalCertManagement": {
Expand All @@ -279,10 +302,11 @@ func TestSetDefaults_Configuration(t *testing.T) {
WebhookServiceName: ptr.To(DefaultWebhookServiceName),
WebhookSecretName: ptr.To(DefaultWebhookSecretName),
},
ClientConnection: defaultClientConnection,
Integrations: overwriteNamespaceIntegrations,
QueueVisibility: defaultQueueVisibility,
MultiKueue: defaultMultiKueue,
ClientConnection: defaultClientConnection,
Integrations: overwriteNamespaceIntegrations,
QueueVisibility: defaultQueueVisibility,
MultiKueue: defaultMultiKueue,
ManagedJobsNamespaceSelector: overwriteNamespaceSelector,
},
},
"should not default InternalCertManagement": {
Expand All @@ -298,10 +322,11 @@ func TestSetDefaults_Configuration(t *testing.T) {
InternalCertManagement: &InternalCertManagement{
Enable: ptr.To(false),
},
ClientConnection: defaultClientConnection,
Integrations: overwriteNamespaceIntegrations,
QueueVisibility: defaultQueueVisibility,
MultiKueue: defaultMultiKueue,
ClientConnection: defaultClientConnection,
Integrations: overwriteNamespaceIntegrations,
QueueVisibility: defaultQueueVisibility,
MultiKueue: defaultMultiKueue,
ManagedJobsNamespaceSelector: overwriteNamespaceSelector,
},
},
"should not default values in custom ClientConnection": {
Expand All @@ -325,9 +350,10 @@ func TestSetDefaults_Configuration(t *testing.T) {
QPS: ptr.To[float32](123.0),
Burst: ptr.To[int32](456),
},
Integrations: overwriteNamespaceIntegrations,
QueueVisibility: defaultQueueVisibility,
MultiKueue: defaultMultiKueue,
Integrations: overwriteNamespaceIntegrations,
QueueVisibility: defaultQueueVisibility,
MultiKueue: defaultMultiKueue,
ManagedJobsNamespaceSelector: overwriteNamespaceSelector,
},
},
"should default empty custom ClientConnection": {
Expand All @@ -344,10 +370,11 @@ func TestSetDefaults_Configuration(t *testing.T) {
InternalCertManagement: &InternalCertManagement{
Enable: ptr.To(false),
},
ClientConnection: defaultClientConnection,
Integrations: overwriteNamespaceIntegrations,
QueueVisibility: defaultQueueVisibility,
MultiKueue: defaultMultiKueue,
ClientConnection: defaultClientConnection,
Integrations: overwriteNamespaceIntegrations,
QueueVisibility: defaultQueueVisibility,
MultiKueue: defaultMultiKueue,
ManagedJobsNamespaceSelector: overwriteNamespaceSelector,
},
},
"defaulting waitForPodsReady values": {
Expand Down Expand Up @@ -375,10 +402,11 @@ func TestSetDefaults_Configuration(t *testing.T) {
InternalCertManagement: &InternalCertManagement{
Enable: ptr.To(false),
},
ClientConnection: defaultClientConnection,
Integrations: defaultIntegrations,
QueueVisibility: defaultQueueVisibility,
MultiKueue: defaultMultiKueue,
ClientConnection: defaultClientConnection,
Integrations: defaultIntegrations,
QueueVisibility: defaultQueueVisibility,
MultiKueue: defaultMultiKueue,
ManagedJobsNamespaceSelector: defaultManagedJobsNamespaceSelector,
},
},
"set waitForPodsReady.blockAdmission to false when enable is false": {
Expand Down Expand Up @@ -406,10 +434,11 @@ func TestSetDefaults_Configuration(t *testing.T) {
InternalCertManagement: &InternalCertManagement{
Enable: ptr.To(false),
},
ClientConnection: defaultClientConnection,
Integrations: defaultIntegrations,
QueueVisibility: defaultQueueVisibility,
MultiKueue: defaultMultiKueue,
ClientConnection: defaultClientConnection,
Integrations: defaultIntegrations,
QueueVisibility: defaultQueueVisibility,
MultiKueue: defaultMultiKueue,
ManagedJobsNamespaceSelector: defaultManagedJobsNamespaceSelector,
},
},
"respecting provided waitForPodsReady values": {
Expand Down Expand Up @@ -443,10 +472,11 @@ func TestSetDefaults_Configuration(t *testing.T) {
InternalCertManagement: &InternalCertManagement{
Enable: ptr.To(false),
},
ClientConnection: defaultClientConnection,
Integrations: defaultIntegrations,
QueueVisibility: defaultQueueVisibility,
MultiKueue: defaultMultiKueue,
ClientConnection: defaultClientConnection,
Integrations: defaultIntegrations,
QueueVisibility: defaultQueueVisibility,
MultiKueue: defaultMultiKueue,
ManagedJobsNamespaceSelector: defaultManagedJobsNamespaceSelector,
},
},
"integrations": {
Expand All @@ -469,8 +499,9 @@ func TestSetDefaults_Configuration(t *testing.T) {
Frameworks: []string{"a", "b"},
PodOptions: defaultIntegrations.PodOptions,
},
QueueVisibility: defaultQueueVisibility,
MultiKueue: defaultMultiKueue,
QueueVisibility: defaultQueueVisibility,
MultiKueue: defaultMultiKueue,
ManagedJobsNamespaceSelector: defaultManagedJobsNamespaceSelector,
},
},
"queue visibility": {
Expand Down Expand Up @@ -499,7 +530,8 @@ func TestSetDefaults_Configuration(t *testing.T) {
MaxCount: 0,
},
},
MultiKueue: defaultMultiKueue,
MultiKueue: defaultMultiKueue,
ManagedJobsNamespaceSelector: defaultManagedJobsNamespaceSelector,
},
},
"multiKueue": {
Expand Down Expand Up @@ -527,6 +559,7 @@ func TestSetDefaults_Configuration(t *testing.T) {
Origin: ptr.To("multikueue-manager1"),
WorkerLostTimeout: &metav1.Duration{Duration: time.Minute},
},
ManagedJobsNamespaceSelector: defaultManagedJobsNamespaceSelector,
},
},
"multiKueue GCInterval 0": {
Expand All @@ -553,6 +586,7 @@ func TestSetDefaults_Configuration(t *testing.T) {
Origin: ptr.To("multikueue-manager1"),
WorkerLostTimeout: &metav1.Duration{Duration: 15 * time.Minute},
},
ManagedJobsNamespaceSelector: defaultManagedJobsNamespaceSelector,
},
},
"add default fair sharing configuration when enabled": {
Expand All @@ -570,10 +604,11 @@ func TestSetDefaults_Configuration(t *testing.T) {
InternalCertManagement: &InternalCertManagement{
Enable: ptr.To(false),
},
ClientConnection: defaultClientConnection,
Integrations: defaultIntegrations,
QueueVisibility: defaultQueueVisibility,
MultiKueue: defaultMultiKueue,
ClientConnection: defaultClientConnection,
Integrations: defaultIntegrations,
QueueVisibility: defaultQueueVisibility,
MultiKueue: defaultMultiKueue,
ManagedJobsNamespaceSelector: defaultManagedJobsNamespaceSelector,
FairSharing: &FairSharing{
Enable: true,
PreemptionStrategies: []PreemptionStrategy{LessThanOrEqualToFinalShare, LessThanInitialShare},
Expand All @@ -598,10 +633,11 @@ func TestSetDefaults_Configuration(t *testing.T) {
InternalCertManagement: &InternalCertManagement{
Enable: ptr.To(false),
},
ClientConnection: defaultClientConnection,
Integrations: defaultIntegrations,
QueueVisibility: defaultQueueVisibility,
MultiKueue: defaultMultiKueue,
ClientConnection: defaultClientConnection,
Integrations: defaultIntegrations,
QueueVisibility: defaultQueueVisibility,
MultiKueue: defaultMultiKueue,
ManagedJobsNamespaceSelector: defaultManagedJobsNamespaceSelector,
Resources: &Resources{
Transformations: []ResourceTransformation{
{Input: corev1.ResourceCPU, Strategy: ptr.To(DefaultResourceTransformationStrategy)},
Expand Down
5 changes: 5 additions & 0 deletions apis/config/v1beta1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions cmd/kueue/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"go.uber.org/zap/zapcore"
corev1 "k8s.io/api/core/v1"
schedulingv1 "k8s.io/api/scheduling/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
"k8s.io/apimachinery/pkg/util/sets"
Expand Down Expand Up @@ -305,6 +306,15 @@ func setupControllers(ctx context.Context, mgr ctrl.Manager, cCache *cache.Cache
jobframework.WithCache(cCache),
jobframework.WithQueues(queues),
}
if features.Enabled(features.ManagedJobsNamespaceSelector) {
nsSelector, err := metav1.LabelSelectorAsSelector(cfg.ManagedJobsNamespaceSelector)
if err != nil {
setupLog.Error(err, "Failed to parse managedJobsNamespaceSelector")
os.Exit(1)
}
opts = append(opts, jobframework.WithManagedJobsNamespaceSelector(nsSelector))
}

if err := jobframework.SetupControllers(ctx, mgr, setupLog, opts...); err != nil {
setupLog.Error(err, "Unable to create controller or webhook", "kubernetesVersion", serverVersionFetcher.GetServerVersion())
os.Exit(1)
Expand Down
3 changes: 3 additions & 0 deletions config/components/manager/controller_manager_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ clientConnection:
# backoffBaseSeconds: 60
# backoffMaxSeconds: 3600
#manageJobsWithoutQueueName: true
#managedJobsNamespaceSelector:
# matchLabels:
# kueue-managed: "true"
#internalCertManagement:
# enable: false
# webhookServiceName: ""
Expand Down
6 changes: 3 additions & 3 deletions keps/3589-manage-jobs-selectively/kep.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ title: Uniformly filter manageJobsWithoutQueueNames by namespace
kep-number: 3589
authors:
- "@dgrove-oss"
status: provisional
status: implementable
creation-date: 2024-11-19
reviewers:
- "@mimowo"
Expand All @@ -16,11 +16,11 @@ stage: beta
# The most recent milestone for which work toward delivery of this KEP has been
# done. This can be the current (upcoming) milestone, if it is being actively
# worked on.
latest-milestone: "v0.11"
latest-milestone: "v0.10"

# The milestone at which this feature was, or is targeted to be, at each stage.
milestone:
beta: "v0.11"
beta: "v0.10"

feature-gates:
- name: ManagedJobsNamespaceSelector
Expand Down
Loading

0 comments on commit 17e0fe8

Please sign in to comment.