Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Ensure nodeSelector logic is consistent for all operators #466

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+276 −37
Open

Ensure nodeSelector logic is consistent for all operators #466

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
98f7e6e
Set nodeSelector on jobs and allow empty nodeSelector
olliewalsh Nov 8, 2024
62c0ff0
Add nodeSelector functional tests
olliewalsh Nov 13, 2024
0fd9df2
Pass-through empty nodeSelector to podSpecs
olliewalsh Nov 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion api/v1beta1/cinder_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ type CinderSpecBase struct {
// NodeSelector to target subset of worker nodes running this service. Setting
// NodeSelector here acts as a default value and can be overridden by service
// specific NodeSelector Settings.
NodeSelector map[string]string `json:"nodeSelector,omitempty"`
NodeSelector *map[string]string `json:"nodeSelector,omitempty"`

// +kubebuilder:validation:Optional
// DBPurge parameters -
Expand Down
2 changes: 1 addition & 1 deletion api/v1beta1/common_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ type CinderServiceTemplate struct {
// +kubebuilder:validation:Optional
// NodeSelector to target subset of worker nodes running this service. Setting here overrides
// any global NodeSelector settings within the Cinder CR.
NodeSelector map[string]string `json:"nodeSelector,omitempty"`
NodeSelector *map[string]string `json:"nodeSelector,omitempty"`

// +kubebuilder:validation:Optional
// CustomServiceConfig - customize the service config using this parameter to change service defaults,
Expand Down
20 changes: 14 additions & 6 deletions api/v1beta1/zz_generated.deepcopy.go
View file
Open in desktop

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

31 changes: 16 additions & 15 deletions controllers/cinder_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1014,6 +1014,10 @@ func (r *CinderReconciler) apiDeploymentCreateOrUpdate(ctx context.Context, inst
ServiceAccount: instance.RbacResourceName(),
}

if cinderAPISpec.NodeSelector == nil {
cinderAPISpec.NodeSelector = instance.Spec.NodeSelector
}

deployment := &cinderv1beta1.CinderAPI{
ObjectMeta: metav1.ObjectMeta{
Name: fmt.Sprintf("%s-api", instance.Name),
Expand All @@ -1023,9 +1027,6 @@ func (r *CinderReconciler) apiDeploymentCreateOrUpdate(ctx context.Context, inst

op, err := controllerutil.CreateOrUpdate(ctx, r.Client, deployment, func() error {
deployment.Spec = cinderAPISpec
if len(deployment.Spec.NodeSelector) == 0 {
deployment.Spec.NodeSelector = instance.Spec.NodeSelector
}

err := controllerutil.SetControllerReference(instance, deployment, r.Scheme)
if err != nil {
Expand All @@ -1049,6 +1050,10 @@ func (r *CinderReconciler) schedulerDeploymentCreateOrUpdate(ctx context.Context
TLS: instance.Spec.CinderAPI.TLS.Ca,
}

if cinderSchedulerSpec.NodeSelector == nil {
cinderSchedulerSpec.NodeSelector = instance.Spec.NodeSelector
}

deployment := &cinderv1beta1.CinderScheduler{
ObjectMeta: metav1.ObjectMeta{
Name: fmt.Sprintf("%s-scheduler", instance.Name),
Expand All @@ -1058,9 +1063,6 @@ func (r *CinderReconciler) schedulerDeploymentCreateOrUpdate(ctx context.Context

op, err := controllerutil.CreateOrUpdate(ctx, r.Client, deployment, func() error {
deployment.Spec = cinderSchedulerSpec
if len(deployment.Spec.NodeSelector) == 0 {
deployment.Spec.NodeSelector = instance.Spec.NodeSelector
}

err := controllerutil.SetControllerReference(instance, deployment, r.Scheme)
if err != nil {
Expand All @@ -1084,6 +1086,10 @@ func (r *CinderReconciler) backupDeploymentCreateOrUpdate(ctx context.Context, i
TLS: instance.Spec.CinderAPI.TLS.Ca,
}

if cinderBackupSpec.NodeSelector == nil {
cinderBackupSpec.NodeSelector = instance.Spec.NodeSelector
}

deployment := &cinderv1beta1.CinderBackup{
ObjectMeta: metav1.ObjectMeta{
Name: fmt.Sprintf("%s-backup", instance.Name),
Expand All @@ -1093,9 +1099,6 @@ func (r *CinderReconciler) backupDeploymentCreateOrUpdate(ctx context.Context, i

op, err := controllerutil.CreateOrUpdate(ctx, r.Client, deployment, func() error {
deployment.Spec = cinderBackupSpec
if len(deployment.Spec.NodeSelector) == 0 {
deployment.Spec.NodeSelector = instance.Spec.NodeSelector
}

err := controllerutil.SetControllerReference(instance, deployment, r.Scheme)
if err != nil {
Expand Down Expand Up @@ -1146,6 +1149,10 @@ func (r *CinderReconciler) volumeDeploymentCreateOrUpdate(ctx context.Context, i
TLS: instance.Spec.CinderAPI.TLS.Ca,
}

if cinderVolumeSpec.CinderVolumeTemplate.NodeSelector == nil {
cinderVolumeSpec.CinderVolumeTemplate.NodeSelector = instance.Spec.NodeSelector
}

deployment := &cinderv1beta1.CinderVolume{
ObjectMeta: metav1.ObjectMeta{
Name: fmt.Sprintf("%s-volume-%s", instance.Name, name),
Expand All @@ -1157,12 +1164,6 @@ func (r *CinderReconciler) volumeDeploymentCreateOrUpdate(ctx context.Context, i
op, err := controllerutil.CreateOrUpdate(ctx, r.Client, deployment, func() error {
deployment.Spec = cinderVolumeSpec

// If NodeSelector is not specified in volumeTemplate, the current
// cinder-volume instance inherits the value from the top-level CR
if len(volTemplate.NodeSelector) == 0 {
deployment.Spec.NodeSelector = instance.Spec.NodeSelector
}

err := controllerutil.SetControllerReference(instance, deployment, r.Scheme)
if err != nil {
return err
Expand Down
6 changes: 4 additions & 2 deletions pkg/cinder/cronjob.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,10 @@ func CronJob(
},
},
}
if instance.Spec.NodeSelector != nil && len(instance.Spec.NodeSelector) > 0 {
cronjob.Spec.JobTemplate.Spec.Template.Spec.NodeSelector = instance.Spec.NodeSelector

if instance.Spec.NodeSelector != nil {
cronjob.Spec.JobTemplate.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector
}

return cronjob
}
4 changes: 4 additions & 0 deletions pkg/cinder/dbsync.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,5 +115,9 @@ func DbSyncJob(instance *cinderv1beta1.Cinder, labels map[string]string, annotat
},
}

if instance.Spec.NodeSelector != nil {
job.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector
}

return job
}
9 changes: 6 additions & 3 deletions pkg/cinderapi/statefuleset.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,13 +165,16 @@ func StatefulSet(
LivenessProbe: livenessProbe,
},
},
Affinity: cinder.GetPodAffinity(ComponentName),
NodeSelector: instance.Spec.NodeSelector,
Volumes: volumes,
Affinity: cinder.GetPodAffinity(ComponentName),
Volumes: volumes,
},
},
},
}

if instance.Spec.NodeSelector != nil {
statefulset.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector
}

return statefulset, nil
}
9 changes: 6 additions & 3 deletions pkg/cinderbackup/statefulset.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,13 +146,16 @@ func StatefulSet(
VolumeMounts: volumeMounts,
},
},
Affinity: cinder.GetPodAffinity(ComponentName),
NodeSelector: instance.Spec.NodeSelector,
Volumes: volumes,
Affinity: cinder.GetPodAffinity(ComponentName),
Volumes: volumes,
},
},
},
}

if instance.Spec.NodeSelector != nil {
statefulset.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector
}

return statefulset
}
9 changes: 6 additions & 3 deletions pkg/cinderscheduler/statefulset.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,13 +131,16 @@ func StatefulSet(
VolumeMounts: volumeMounts,
},
},
Affinity: cinder.GetPodAffinity(ComponentName),
NodeSelector: instance.Spec.NodeSelector,
Volumes: volumes,
Affinity: cinder.GetPodAffinity(ComponentName),
Volumes: volumes,
},
},
},
}

if instance.Spec.NodeSelector != nil {
statefulset.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector
}

return statefulset
}
9 changes: 6 additions & 3 deletions pkg/cindervolume/statefulset.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,13 +153,16 @@ func StatefulSet(
VolumeMounts: volumeMounts,
},
},
Affinity: cinder.GetPodAffinity(ComponentName),
NodeSelector: instance.Spec.NodeSelector,
Volumes: volumes,
Affinity: cinder.GetPodAffinity(ComponentName),
Volumes: volumes,
},
},
},
}

if instance.Spec.NodeSelector != nil {
statefulset.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector
}

return statefulset
}
9 changes: 9 additions & 0 deletions test/functional/base_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"golang.org/x/exp/maps"

. "github.com/onsi/gomega" //revive:disable:dot-imports
batchv1 "k8s.io/api/batch/v1"
corev1 "k8s.io/api/core/v1"
k8s_errors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
Expand Down Expand Up @@ -230,6 +231,14 @@ func GetCinderVolume(name types.NamespacedName) *cinderv1.CinderVolume {
return instance
}

func GetCronJob(name types.NamespacedName) *batchv1.CronJob {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in a follow up should add this to https://github.com/openstack-k8s-operators/lib-common/tree/main/modules/common/test/functional

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, also for keystone where this came from

instance := &batchv1.CronJob{}
Eventually(func(g Gomega) {
g.Expect(k8sClient.Get(ctx, name, instance)).Should(Succeed())
}, timeout, interval).Should(Succeed())
return instance
}

func CinderAPIConditionGetter(name types.NamespacedName) condition.Conditions {
instance := GetCinderAPI(name)
return instance.Status.Conditions
Expand Down
Loading
Loading