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

Apply mc-name-suffix annotation to kubeletconfig #3353

Closed
wants to merge 3 commits into from
Closed

Apply mc-name-suffix annotation to kubeletconfig #3353

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
ae87b31
Apply mc-name-suffix annotation to kubeletconfig
SrinivasAtmakuri Jan 15, 2024
baecdcc
updates
hawkowl Jan 18, 2024
9af15a0
revert systemreserved changes
hawkowl Jan 18, 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
53 changes: 52 additions & 1 deletion pkg/operator/controllers/autosizednodes/autosizednodes_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ type Reconciler struct {
const (
ControllerName = "AutoSizedNodes"
configName = "dynamic-node"

// machineconfiguration annotation added on the kubelet CRD - dynamic-node
mcAnnotationName = "machineconfiguration.openshift.io/mc-name-suffix"
)

func NewReconciler(log *logrus.Entry, client client.Client) *Reconciler {
Expand Down Expand Up @@ -90,7 +93,55 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.
return ctrl.Result{}, fmt.Errorf("could not fetch KubeletConfig: %w", err)
}

// If already exists, update the spec
// Fetch the existing KubeletConfigs
var kubeletConfigs mcv1.KubeletConfigList
err = r.client.List(ctx, &kubeletConfigs)
if err != nil {
// If error, return it (controller-runtime will requeue for a retry)
return ctrl.Result{}, fmt.Errorf("could not fetch KubeletConfig list: %w", err)
}

// If we have a single KubeletConfig (ours), verify that the mc-name-suffix
// annotation is present. This will automatically be set on newer (4.11+?)
// OpenShift versions. If we are on an older version then we should set our
// KubeletConfig with the empty string so that it is equivalent to suffix 0,
// so upgrades will put customer KubeletConfigs at higher indexes.
if len(kubeletConfigs.Items) == 1 {
if len(config.Annotations) == 0 {
// no annotations, set it to empty string (equiv to suffix 0)
config.Annotations = map[string]string{
mcAnnotationName: "",
}
} else {
if _, ok := config.Annotations[mcAnnotationName]; !ok {
// if the annotation is not present, set it to the empty string (equiv to suffix 0)
config.Annotations[mcAnnotationName] = ""
}
}
} else {
// Check if any of the KubeletConfigs have the mc-name-suffix annotation set
var annotationsPresent bool
for _, i := range kubeletConfigs.Items {
if i.ObjectMeta.Name != configName {
if _, ok := i.Annotations[mcAnnotationName]; ok {
annotationsPresent = true
}
}
}

// If no KubeletConfigs have the annotation set, it is before OpenShift
// utilises it, and it is safe to mark our one with the empty string
if !annotationsPresent {
if config.Annotations == nil {
config.Annotations = map[string]string{
mcAnnotationName: "",
}
} else {
config.Annotations[mcAnnotationName] = ""
}
}
}

config.Spec = defaultConfig.Spec
err = r.client.Update(ctx, &config)
if err != nil {
Expand Down
93 changes: 89 additions & 4 deletions pkg/operator/controllers/autosizednodes/autosizednodes_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,27 @@ func TestAutosizednodesReconciler(t *testing.T) {
emptyConfig := mcv1.KubeletConfig{}
config := makeConfig()

configWithOtherAnnotations := makeConfig()
configWithOtherAnnotations.Annotations = map[string]string{
"some-other-annotation": "value",
}

configWithOtherAnnotationsAndMCO := makeConfig()
configWithOtherAnnotationsAndMCO.Annotations = map[string]string{
"some-other-annotation": "value",
"machineconfiguration.openshift.io/mc-name-suffix": "",
}

configWithAnnotation := makeConfig()
configWithAnnotation.Annotations = map[string]string{
"machineconfiguration.openshift.io/mc-name-suffix": "",
}

configWithHighAnnotation := makeConfig()
configWithHighAnnotation.Annotations = map[string]string{
"machineconfiguration.openshift.io/mc-name-suffix": "3",
}

tests := []struct {
name string
wantErrMsg string
Expand All @@ -59,14 +80,15 @@ func TestAutosizednodesReconciler(t *testing.T) {
wantErrMsg: kerrors.NewNotFound(mcv1.Resource("kubeletconfigs"), "dynamic-node").Error(),
},
{
name: "is needed and not present already",
client: fake.NewClientBuilder().WithRuntimeObjects(aro(true)).Build(),
name: "is needed and not present already",
client: fake.NewClientBuilder().WithRuntimeObjects(aro(true)).Build(),
// We don't create the KubeletConfig with the annotation because it gets updated by MCO, and we
wantConfig: &config,
},
{
name: "is needed and present already",
client: fake.NewClientBuilder().WithRuntimeObjects(aro(true), &config).Build(),
wantConfig: &config,
client: fake.NewClientBuilder().WithRuntimeObjects(aro(true), &configWithAnnotation).Build(),
wantConfig: &configWithAnnotation,
},
{
name: "is not needed and is present",
Expand All @@ -81,6 +103,9 @@ func TestAutosizednodesReconciler(t *testing.T) {
&mcv1.KubeletConfig{
ObjectMeta: metav1.ObjectMeta{
Name: configName,
Annotations: map[string]string{
"machineconfiguration.openshift.io/mc-name-suffix": "",
},
},
Spec: mcv1.KubeletConfigSpec{
AutoSizingReserved: to.BoolPtr(false),
Expand All @@ -94,6 +119,63 @@ func TestAutosizednodesReconciler(t *testing.T) {
},
},
}).Build(),
wantConfig: &configWithAnnotation,
},
{
name: "is needed and present already, only kubelet config, gets MCO annotation set",
client: fake.NewClientBuilder().WithRuntimeObjects(aro(true), &config).Build(),
wantConfig: &configWithAnnotation,
},
{
name: "is needed and present already, non-nil annotations, only kubelet config, gets MCO annotation set",
client: fake.NewClientBuilder().WithRuntimeObjects(aro(true), &configWithOtherAnnotations).Build(),
wantConfig: &configWithOtherAnnotationsAndMCO,
},
{
name: "is needed and present already, only kubelet config, has MCO annotation already, annotation remains",
client: fake.NewClientBuilder().WithRuntimeObjects(aro(true), &configWithHighAnnotation).Build(),
wantConfig: &configWithHighAnnotation,
},
{
name: "is needed and present already, several kubelet configs without MCO annotations, gets MCO annotation set",
client: fake.NewClientBuilder().WithRuntimeObjects(
aro(true),
&config,
&mcv1.KubeletConfig{
ObjectMeta: metav1.ObjectMeta{
Name: "some-other-config",
},
Spec: mcv1.KubeletConfigSpec{},
}).Build(),
wantConfig: &configWithAnnotation,
},
{
name: "is needed and present already, annotations present, several kubelet configs without MCO annotations, gets MCO annotation set",
client: fake.NewClientBuilder().WithRuntimeObjects(
aro(true),
&configWithOtherAnnotations,
&mcv1.KubeletConfig{
ObjectMeta: metav1.ObjectMeta{
Name: "some-other-config",
},
Spec: mcv1.KubeletConfigSpec{},
}).Build(),
wantConfig: &configWithOtherAnnotationsAndMCO,
},
{
name: "is needed and present already, several kubelet configs, annotations exist, MCO annotation not set",
client: fake.NewClientBuilder().WithRuntimeObjects(
aro(true),
&config,
&mcv1.KubeletConfig{
ObjectMeta: metav1.ObjectMeta{
Name: "some-other-config",
Annotations: map[string]string{
"machineconfiguration.openshift.io/mc-name-suffix": "",
},
},
Spec: mcv1.KubeletConfigSpec{},
}).Build(),
wantConfig: &config,
},
}
Expand Down Expand Up @@ -121,6 +203,9 @@ func TestAutosizednodesReconciler(t *testing.T) {
if !reflect.DeepEqual(test.wantConfig.Spec, c.Spec) {
t.Error(cmp.Diff(test.wantConfig.Spec, c.Spec))
}
if !reflect.DeepEqual(test.wantConfig.Annotations, c.Annotations) {
t.Error(cmp.Diff(test.wantConfig.Annotations, c.Annotations))
}

if result != (ctrl.Result{}) {
t.Error("reconcile returned an unexpected result")
Expand Down
Loading