From e65917a873aec8d7a9e5ecf0bd64434fc59d2b23 Mon Sep 17 00:00:00 2001 From: Matthew Kenigsberg Date: Thu, 16 Dec 2021 10:31:48 -0600 Subject: [PATCH] Use library-go ApplyDeployment and ApplyDaemonset Our current functions in ./lib are out of date and we should be using library-go instead The library-go functions require passing the generation of the previously applied deployment/daemonset, and this should be stored on an MCO CRD. Because this currently doesn't exist, the existing generation is passed as generation. This is no worse than how MCO's current functions work, so the library-go functions should still be used until an MCO CRD is created. Files with the now unused functions and unused helper functions were removed: resourceapply/apps.go resourcemerge/{apps.go,apps_test.go,core.go,core_test.go} Helps https://github.com/openshift/machine-config-operator/issues/819 --- lib/resourceapply/apps.go | 52 ---- lib/resourcemerge/apps.go | 45 --- lib/resourcemerge/apps_test.go | 147 ---------- lib/resourcemerge/core.go | 508 --------------------------------- lib/resourcemerge/core_test.go | 133 --------- pkg/operator/sync.go | 18 +- 6 files changed, 16 insertions(+), 887 deletions(-) delete mode 100644 lib/resourceapply/apps.go delete mode 100644 lib/resourcemerge/apps.go delete mode 100644 lib/resourcemerge/apps_test.go delete mode 100644 lib/resourcemerge/core.go delete mode 100644 lib/resourcemerge/core_test.go diff --git a/lib/resourceapply/apps.go b/lib/resourceapply/apps.go deleted file mode 100644 index 3bcd8e9b12..0000000000 --- a/lib/resourceapply/apps.go +++ /dev/null @@ -1,52 +0,0 @@ -package resourceapply - -import ( - "context" - "github.com/openshift/machine-config-operator/lib/resourcemerge" - appsv1 "k8s.io/api/apps/v1" - apierrors "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - appsclientv1 "k8s.io/client-go/kubernetes/typed/apps/v1" -) - -// ApplyDaemonSet applies the required daemonset to the cluster. -func ApplyDaemonSet(client appsclientv1.DaemonSetsGetter, required *appsv1.DaemonSet) (*appsv1.DaemonSet, bool, error) { - existing, err := client.DaemonSets(required.Namespace).Get(context.TODO(), required.Name, metav1.GetOptions{}) - if apierrors.IsNotFound(err) { - actual, err := client.DaemonSets(required.Namespace).Create(context.TODO(), required, metav1.CreateOptions{}) - return actual, true, err - } - if err != nil { - return nil, false, err - } - - modified := resourcemerge.BoolPtr(false) - resourcemerge.EnsureDaemonSet(modified, existing, *required) - if !*modified { - return existing, false, nil - } - - actual, err := client.DaemonSets(required.Namespace).Update(context.TODO(), existing, metav1.UpdateOptions{}) - return actual, true, err -} - -// ApplyDeployment applies the required deployment to the cluster. -func ApplyDeployment(client appsclientv1.DeploymentsGetter, required *appsv1.Deployment) (*appsv1.Deployment, bool, error) { - existing, err := client.Deployments(required.Namespace).Get(context.TODO(), required.Name, metav1.GetOptions{}) - if apierrors.IsNotFound(err) { - actual, err := client.Deployments(required.Namespace).Create(context.TODO(), required, metav1.CreateOptions{}) - return actual, true, err - } - if err != nil { - return nil, false, err - } - - modified := resourcemerge.BoolPtr(false) - resourcemerge.EnsureDeployment(modified, existing, *required) - if !*modified { - return existing, false, nil - } - - actual, err := client.Deployments(required.Namespace).Update(context.TODO(), existing, metav1.UpdateOptions{}) - return actual, true, err -} diff --git a/lib/resourcemerge/apps.go b/lib/resourcemerge/apps.go deleted file mode 100644 index 3cc240e08b..0000000000 --- a/lib/resourcemerge/apps.go +++ /dev/null @@ -1,45 +0,0 @@ -package resourcemerge - -import ( - appsv1 "k8s.io/api/apps/v1" - "k8s.io/apimachinery/pkg/api/equality" -) - -// EnsureDeployment ensures that the existing matches the required. -// modified is set to true when existing had to be updated with required. -func EnsureDeployment(modified *bool, existing *appsv1.Deployment, required appsv1.Deployment) { - EnsureObjectMeta(modified, &existing.ObjectMeta, required.ObjectMeta) - - if existing.Spec.Selector == nil { - *modified = true - existing.Spec.Selector = required.Spec.Selector - } - if !equality.Semantic.DeepEqual(existing.Spec.Selector, required.Spec.Selector) { - *modified = true - existing.Spec.Selector = required.Spec.Selector - } - - ensurePodTemplateSpec(modified, &existing.Spec.Template, required.Spec.Template) -} - -// EnsureDaemonSet ensures that the existing matches the required. -// modified is set to true when existing had to be updated with required. -func EnsureDaemonSet(modified *bool, existing *appsv1.DaemonSet, required appsv1.DaemonSet) { - EnsureObjectMeta(modified, &existing.ObjectMeta, required.ObjectMeta) - - if existing.Spec.Selector == nil { - *modified = true - existing.Spec.Selector = required.Spec.Selector - } - if !equality.Semantic.DeepEqual(existing.Spec.Selector, required.Spec.Selector) { - *modified = true - existing.Spec.Selector = required.Spec.Selector - } - - if !equality.Semantic.DeepEqual(existing.Spec.UpdateStrategy, required.Spec.UpdateStrategy) { - *modified = true - existing.Spec.UpdateStrategy = required.Spec.UpdateStrategy - } - - ensurePodTemplateSpec(modified, &existing.Spec.Template, required.Spec.Template) -} diff --git a/lib/resourcemerge/apps_test.go b/lib/resourcemerge/apps_test.go deleted file mode 100644 index ec4902b44b..0000000000 --- a/lib/resourcemerge/apps_test.go +++ /dev/null @@ -1,147 +0,0 @@ -package resourcemerge - -import ( - "fmt" - "testing" - - appsv1 "k8s.io/api/apps/v1" - "k8s.io/apimachinery/pkg/api/equality" - "k8s.io/apimachinery/pkg/util/intstr" -) - -func TestMergeDaemonSetUpdateStrategy(t *testing.T) { - // The merge functions set "modified" to true if pointer fields - // are nil. Since this test doesn't "zero" all possible pointer - // fields, modified will be true, so this test expects this - // behavior - daemonset_update_strategy_tests := []struct { - existing appsv1.DaemonSet - input appsv1.DaemonSet - - expectedModified bool - expected appsv1.DaemonSet - }{ - - { - existing: appsv1.DaemonSet{ - Spec: appsv1.DaemonSetSpec{ - UpdateStrategy: appsv1.DaemonSetUpdateStrategy{ - Type: "RollingUpdate", - RollingUpdate: &appsv1.RollingUpdateDaemonSet{ - MaxUnavailable: &intstr.IntOrString{Type: 0, IntVal: 1, StrVal: ""}, - MaxSurge: &intstr.IntOrString{Type: 0, IntVal: 1, StrVal: ""}, - }, - }, - }, - }, - input: appsv1.DaemonSet{ - Spec: appsv1.DaemonSetSpec{ - UpdateStrategy: appsv1.DaemonSetUpdateStrategy{ - Type: "RollingUpdate", - RollingUpdate: &appsv1.RollingUpdateDaemonSet{ - MaxUnavailable: &intstr.IntOrString{Type: 0, IntVal: 1, StrVal: ""}, - MaxSurge: &intstr.IntOrString{Type: 0, IntVal: 1, StrVal: ""}, - }, - }, - }, - }, - // this "true" is expected true because of the nil pointers elsewhere in the DaemonSet struct - // it always takes new struct and sets modified to "true" in event of nil - expectedModified: true, - expected: appsv1.DaemonSet{ - Spec: appsv1.DaemonSetSpec{ - UpdateStrategy: appsv1.DaemonSetUpdateStrategy{ - Type: "RollingUpdate", - RollingUpdate: &appsv1.RollingUpdateDaemonSet{ - MaxUnavailable: &intstr.IntOrString{Type: 0, IntVal: 1, StrVal: ""}, - MaxSurge: &intstr.IntOrString{Type: 0, IntVal: 1, StrVal: ""}, - }, - }, - }, - }, - }, { - existing: appsv1.DaemonSet{ - Spec: appsv1.DaemonSetSpec{ - UpdateStrategy: appsv1.DaemonSetUpdateStrategy{ - Type: "OnDelete", - }, - }, - }, - input: appsv1.DaemonSet{ - Spec: appsv1.DaemonSetSpec{ - UpdateStrategy: appsv1.DaemonSetUpdateStrategy{ - Type: "RollingUpdate", - RollingUpdate: &appsv1.RollingUpdateDaemonSet{ - MaxUnavailable: &intstr.IntOrString{Type: 0, IntVal: 1, StrVal: ""}, - MaxSurge: &intstr.IntOrString{Type: 0, IntVal: 1, StrVal: ""}, - }, - }, - }, - }, - - expectedModified: true, - expected: appsv1.DaemonSet{ - Spec: appsv1.DaemonSetSpec{ - UpdateStrategy: appsv1.DaemonSetUpdateStrategy{ - Type: "RollingUpdate", - RollingUpdate: &appsv1.RollingUpdateDaemonSet{ - MaxUnavailable: &intstr.IntOrString{Type: 0, IntVal: 1, StrVal: ""}, - MaxSurge: &intstr.IntOrString{Type: 0, IntVal: 1, StrVal: ""}, - }, - }, - }, - }, - }, - { - existing: appsv1.DaemonSet{ - Spec: appsv1.DaemonSetSpec{ - UpdateStrategy: appsv1.DaemonSetUpdateStrategy{ - Type: "RollingUpdate", - RollingUpdate: &appsv1.RollingUpdateDaemonSet{ - MaxUnavailable: &intstr.IntOrString{Type: 0, IntVal: 1, StrVal: ""}, - MaxSurge: &intstr.IntOrString{Type: 0, IntVal: 1, StrVal: ""}, - }, - }, - }, - }, - input: appsv1.DaemonSet{ - Spec: appsv1.DaemonSetSpec{ - UpdateStrategy: appsv1.DaemonSetUpdateStrategy{ - Type: "RollingUpdate", - RollingUpdate: &appsv1.RollingUpdateDaemonSet{ - MaxUnavailable: &intstr.IntOrString{Type: 0, IntVal: 2, StrVal: ""}, - MaxSurge: &intstr.IntOrString{Type: 0, IntVal: 3, StrVal: ""}, - }, - }, - }, - }, - expectedModified: true, - expected: appsv1.DaemonSet{ - Spec: appsv1.DaemonSetSpec{ - UpdateStrategy: appsv1.DaemonSetUpdateStrategy{ - Type: "RollingUpdate", - RollingUpdate: &appsv1.RollingUpdateDaemonSet{ - MaxUnavailable: &intstr.IntOrString{Type: 0, IntVal: 2, StrVal: ""}, - MaxSurge: &intstr.IntOrString{Type: 0, IntVal: 3, StrVal: ""}, - }, - }, - }, - }, - }, - } - - for idx, test := range daemonset_update_strategy_tests { - t.Run(fmt.Sprintf("test#%d", idx), func(t *testing.T) { - modified := BoolPtr(false) - EnsureDaemonSet(modified, &test.existing, test.input) - - if *modified != test.expectedModified { - t.Fatalf("mismatch updatestrategy got: %v want: %v", *modified, test.expectedModified) - } - - if !equality.Semantic.DeepEqual(test.existing, test.expected) { - t.Fatalf("mismatch updatestrategy got: %v want: %v", test.existing, test.expected) - } - }) - } -} diff --git a/lib/resourcemerge/core.go b/lib/resourcemerge/core.go deleted file mode 100644 index bd4fdcca4e..0000000000 --- a/lib/resourcemerge/core.go +++ /dev/null @@ -1,508 +0,0 @@ -package resourcemerge - -import ( - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/equality" -) - -// EnsureConfigMap ensures that the existing matches the required. -// modified is set to true when existing had to be updated with required. -func EnsureConfigMap(modified *bool, existing *corev1.ConfigMap, required corev1.ConfigMap) { - EnsureObjectMeta(modified, &existing.ObjectMeta, required.ObjectMeta) - - mergeMap(modified, &existing.Data, required.Data) -} - -// ensurePodTemplateSpec ensures that the existing matches the required. -// modified is set to true when existing had to be updated with required. -func ensurePodTemplateSpec(modified *bool, existing *corev1.PodTemplateSpec, required corev1.PodTemplateSpec) { - EnsureObjectMeta(modified, &existing.ObjectMeta, required.ObjectMeta) - - ensurePodSpec(modified, &existing.Spec, required.Spec) -} - -func ensurePodSpec(modified *bool, existing *corev1.PodSpec, required corev1.PodSpec) { - // any container we specify, we require. - for _, required := range required.InitContainers { - var existingCurr *corev1.Container - for j, curr := range existing.InitContainers { - if curr.Name == required.Name { - existingCurr = &existing.InitContainers[j] - break - } - } - if existingCurr == nil { - *modified = true - existing.Containers = append(existing.InitContainers, corev1.Container{}) - existingCurr = &existing.InitContainers[len(existing.InitContainers)-1] - } - ensureContainer(modified, existingCurr, required) - } - - for _, required := range required.Containers { - var existingCurr *corev1.Container - for j, curr := range existing.Containers { - if curr.Name == required.Name { - existingCurr = &existing.Containers[j] - break - } - } - if existingCurr == nil { - *modified = true - existing.Containers = append(existing.Containers, corev1.Container{}) - existingCurr = &existing.Containers[len(existing.Containers)-1] - } - ensureContainer(modified, existingCurr, required) - } - - // any volume we specify, we require. - for _, required := range required.Volumes { - var existingCurr *corev1.Volume - for j, curr := range existing.Volumes { - if curr.Name == required.Name { - existingCurr = &existing.Volumes[j] - break - } - } - if existingCurr == nil { - *modified = true - existing.Volumes = append(existing.Volumes, corev1.Volume{}) - existingCurr = &existing.Volumes[len(existing.Volumes)-1] - } - ensureVolume(modified, existingCurr, required) - } - - if len(required.RestartPolicy) > 0 { - if existing.RestartPolicy != required.RestartPolicy { - *modified = true - existing.RestartPolicy = required.RestartPolicy - } - } - - setStringIfSet(modified, &existing.ServiceAccountName, required.ServiceAccountName) - setBool(modified, &existing.HostNetwork, required.HostNetwork) - mergeMap(modified, &existing.NodeSelector, required.NodeSelector) - ensurePodSecurityContextPtr(modified, &existing.SecurityContext, required.SecurityContext) - ensureAffinityPtr(modified, &existing.Affinity, required.Affinity) - ensureTolerations(modified, &existing.Tolerations, required.Tolerations) - setStringIfSet(modified, &existing.PriorityClassName, required.PriorityClassName) - setInt32Ptr(modified, &existing.Priority, required.Priority) -} - -func ensureContainer(modified *bool, existing *corev1.Container, required corev1.Container) { - setStringIfSet(modified, &existing.Name, required.Name) - setStringIfSet(modified, &existing.Image, required.Image) - - // if you want modify the launch, you need to modify it in the config, not in the launch args - setStringSliceIfSet(modified, &existing.Command, required.Command) - setStringSliceIfSet(modified, &existing.Args, required.Args) - - setStringIfSet(modified, &existing.WorkingDir, required.WorkingDir) - - // also sync the env vars here, added to handle proxy - // use a map to keep track of removed vars, with empty values - requiredVars := make(map[string]struct{}) - for _, required := range required.Env { - requiredVars[required.Name] = struct{}{} - var existingCurr *corev1.EnvVar - for j, curr := range existing.Env { - if curr.Name == required.Name { - existingCurr = &existing.Env[j] - break - } - } - if existingCurr == nil { - *modified = true - existing.Env = append(existing.Env, corev1.EnvVar{}) - existingCurr = &existing.Env[len(existing.Env)-1] - } - ensureEnvVar(modified, existingCurr, required) - } - - // any env vars we don't have anymore should be removed - for i := len(existing.Env) - 1; i >= 0; i-- { - if _, ok := requiredVars[existing.Env[i].Name]; !ok { - *modified = true - existing.Env = append(existing.Env[:i], existing.Env[i+1:]...) - } - } - - // any port we specify, we require - for _, required := range required.Ports { - var existingCurr *corev1.ContainerPort - for j, curr := range existing.Ports { - if curr.Name == required.Name { - existingCurr = &existing.Ports[j] - break - } - } - if existingCurr == nil { - *modified = true - existing.Ports = append(existing.Ports, corev1.ContainerPort{}) - existingCurr = &existing.Ports[len(existing.Ports)-1] - } - ensureContainerPort(modified, existingCurr, required) - } - - // any volume mount we specify, we require - for _, required := range required.VolumeMounts { - var existingCurr *corev1.VolumeMount - for j, curr := range existing.VolumeMounts { - if curr.Name == required.Name { - existingCurr = &existing.VolumeMounts[j] - break - } - } - if existingCurr == nil { - *modified = true - existing.VolumeMounts = append(existing.VolumeMounts, corev1.VolumeMount{}) - existingCurr = &existing.VolumeMounts[len(existing.VolumeMounts)-1] - } - ensureVolumeMount(modified, existingCurr, required) - } - - if required.LivenessProbe != nil { - ensureProbePtr(modified, &existing.LivenessProbe, required.LivenessProbe) - } - if required.ReadinessProbe != nil { - ensureProbePtr(modified, &existing.ReadinessProbe, required.ReadinessProbe) - } - - // our security context should always win - ensureSecurityContextPtr(modified, &existing.SecurityContext, required.SecurityContext) -} - -func ensureProbePtr(modified *bool, existing **corev1.Probe, required *corev1.Probe) { - // if we have no required, then we don't care what someone else has set - if required == nil { - return - } - if *existing == nil { - *modified = true - *existing = required - return - } - ensureProbe(modified, *existing, *required) -} - -func ensureProbe(modified *bool, existing *corev1.Probe, required corev1.Probe) { - setInt32(modified, &existing.InitialDelaySeconds, required.InitialDelaySeconds) - - ensureProbeHandler(modified, &existing.ProbeHandler, required.ProbeHandler) -} - -func ensureProbeHandler(modified *bool, existing *corev1.ProbeHandler, required corev1.ProbeHandler) { - if !equality.Semantic.DeepEqual(required, *existing) { - *modified = true - *existing = required - } -} - -func ensureContainerPort(modified *bool, existing *corev1.ContainerPort, required corev1.ContainerPort) { - if !equality.Semantic.DeepEqual(required, *existing) { - *modified = true - *existing = required - } -} - -func ensureEnvVar(modified *bool, existing *corev1.EnvVar, required corev1.EnvVar) { - if !equality.Semantic.DeepEqual(required, *existing) { - *modified = true - *existing = required - } -} - -func ensureVolumeMount(modified *bool, existing *corev1.VolumeMount, required corev1.VolumeMount) { - if !equality.Semantic.DeepEqual(required, *existing) { - *modified = true - *existing = required - } -} - -func ensureVolume(modified *bool, existing *corev1.Volume, required corev1.Volume) { - if !equality.Semantic.DeepEqual(required, *existing) { - *modified = true - *existing = required - } -} - -func ensureSecurityContextPtr(modified *bool, existing **corev1.SecurityContext, required *corev1.SecurityContext) { - // if we have no required, then we don't care what someone else has set - if required == nil { - return - } - - if *existing == nil { - *modified = true - *existing = required - return - } - ensureSecurityContext(modified, *existing, *required) -} - -func ensureSecurityContext(modified *bool, existing *corev1.SecurityContext, required corev1.SecurityContext) { - ensureCapabilitiesPtr(modified, &existing.Capabilities, required.Capabilities) - ensureSELinuxOptionsPtr(modified, &existing.SELinuxOptions, required.SELinuxOptions) - setBoolPtr(modified, &existing.Privileged, required.Privileged) - setInt64Ptr(modified, &existing.RunAsUser, required.RunAsUser) - setBoolPtr(modified, &existing.RunAsNonRoot, required.RunAsNonRoot) - setBoolPtr(modified, &existing.ReadOnlyRootFilesystem, required.ReadOnlyRootFilesystem) - setBoolPtr(modified, &existing.AllowPrivilegeEscalation, required.AllowPrivilegeEscalation) -} - -func ensureCapabilitiesPtr(modified *bool, existing **corev1.Capabilities, required *corev1.Capabilities) { - // if we have no required, then we don't care what someone else has set - if required == nil { - return - } - - if *existing == nil { - *modified = true - *existing = required - return - } - ensureCapabilities(modified, *existing, *required) -} - -func ensureCapabilities(modified *bool, existing *corev1.Capabilities, required corev1.Capabilities) { - // any Add we specify, we require. - for _, required := range required.Add { - found := false - for _, curr := range existing.Add { - if equality.Semantic.DeepEqual(curr, required) { - found = true - break - } - } - if !found { - *modified = true - existing.Add = append(existing.Add, required) - } - } - - // any Drop we specify, we require. - for _, required := range required.Drop { - found := false - for _, curr := range existing.Drop { - if equality.Semantic.DeepEqual(curr, required) { - found = true - break - } - } - if !found { - *modified = true - existing.Drop = append(existing.Drop, required) - } - } -} - -func setStringSliceIfSet(modified *bool, existing *[]string, required []string) { - if required == nil { - return - } - if !equality.Semantic.DeepEqual(required, *existing) { - *existing = required - *modified = true - } -} - -func mergeStringSlice(modified *bool, existing *[]string, required []string) { - for _, required := range required { - found := false - for _, curr := range *existing { - if required == curr { - found = true - break - } - } - if !found { - *modified = true - *existing = append(*existing, required) - } - } -} - -func ensureTolerations(modified *bool, existing *[]corev1.Toleration, required []corev1.Toleration) { - for ridx := range required { - found := false - for eidx := range *existing { - if required[ridx].Key == (*existing)[eidx].Key { - found = true - if !equality.Semantic.DeepEqual((*existing)[eidx], required[ridx]) { - *modified = true - (*existing)[eidx] = required[ridx] - } - break - } - } - if !found { - *modified = true - *existing = append(*existing, required[ridx]) - } - } -} - -func ensureAffinityPtr(modified *bool, existing **corev1.Affinity, required *corev1.Affinity) { - // if we have no required, then we don't care what someone else has set - if required == nil { - return - } - - if *existing == nil { - *modified = true - *existing = required - return - } - ensureAffinity(modified, *existing, *required) -} - -func ensureAffinity(modified *bool, existing *corev1.Affinity, required corev1.Affinity) { - if !equality.Semantic.DeepEqual(existing.NodeAffinity, required.NodeAffinity) { - *modified = true - (*existing).NodeAffinity = required.NodeAffinity - } - if !equality.Semantic.DeepEqual(existing.PodAffinity, required.PodAffinity) { - *modified = true - (*existing).PodAffinity = required.PodAffinity - } - if !equality.Semantic.DeepEqual(existing.PodAntiAffinity, required.PodAntiAffinity) { - *modified = true - (*existing).PodAntiAffinity = required.PodAntiAffinity - } -} - -func ensurePodSecurityContextPtr(modified *bool, existing **corev1.PodSecurityContext, required *corev1.PodSecurityContext) { - // if we have no required, then we don't care what someone else has set - if required == nil { - return - } - - if *existing == nil { - *modified = true - *existing = required - return - } - ensurePodSecurityContext(modified, *existing, *required) -} - -func ensurePodSecurityContext(modified *bool, existing *corev1.PodSecurityContext, required corev1.PodSecurityContext) { - ensureSELinuxOptionsPtr(modified, &existing.SELinuxOptions, required.SELinuxOptions) - setInt64Ptr(modified, &existing.RunAsUser, required.RunAsUser) - setInt64Ptr(modified, &existing.RunAsGroup, required.RunAsGroup) - setBoolPtr(modified, &existing.RunAsNonRoot, required.RunAsNonRoot) - - // any SupplementalGroups we specify, we require. - for _, required := range required.SupplementalGroups { - found := false - for _, curr := range existing.SupplementalGroups { - if curr == required { - found = true - break - } - } - if !found { - *modified = true - existing.SupplementalGroups = append(existing.SupplementalGroups, required) - } - } - - setInt64Ptr(modified, &existing.FSGroup, required.FSGroup) - - // any SupplementalGroups we specify, we require. - for _, required := range required.Sysctls { - found := false - for j, curr := range existing.Sysctls { - if curr.Name == required.Name { - found = true - if curr.Value != required.Value { - *modified = true - existing.Sysctls[j] = required - } - break - } - } - if !found { - *modified = true - existing.Sysctls = append(existing.Sysctls, required) - } - } -} - -func ensureSELinuxOptionsPtr(modified *bool, existing **corev1.SELinuxOptions, required *corev1.SELinuxOptions) { - // if we have no required, then we don't care what someone else has set - if required == nil { - return - } - - if *existing == nil { - *modified = true - *existing = required - return - } - ensureSELinuxOptions(modified, *existing, *required) -} - -func ensureSELinuxOptions(modified *bool, existing *corev1.SELinuxOptions, required corev1.SELinuxOptions) { - setStringIfSet(modified, &existing.User, required.User) - setStringIfSet(modified, &existing.Role, required.Role) - setStringIfSet(modified, &existing.Type, required.Type) - setStringIfSet(modified, &existing.Level, required.Level) -} - -func setBool(modified *bool, existing *bool, required bool) { - if required != *existing { - *existing = required - *modified = true - } -} - -func setBoolPtr(modified *bool, existing **bool, required *bool) { - // if we have no required, then we don't care what someone else has set - if required == nil { - return - } - - if *existing == nil { - *modified = true - *existing = required - return - } - setBool(modified, *existing, *required) -} - -func setInt32(modified *bool, existing *int32, required int32) { - if required != *existing { - *existing = required - *modified = true - } -} - -func setInt32Ptr(modified *bool, existing **int32, required *int32) { - if *existing == nil || (required == nil && *existing != nil) { - *modified = true - *existing = required - return - } - setInt32(modified, *existing, *required) -} - -func setInt64(modified *bool, existing *int64, required int64) { - if required != *existing { - *existing = required - *modified = true - } -} - -func setInt64Ptr(modified *bool, existing **int64, required *int64) { - // if we have no required, then we don't care what someone else has set - if required == nil { - return - } - - if *existing == nil { - *modified = true - *existing = required - return - } - setInt64(modified, *existing, *required) -} diff --git a/lib/resourcemerge/core_test.go b/lib/resourcemerge/core_test.go deleted file mode 100644 index 6a495a1d60..0000000000 --- a/lib/resourcemerge/core_test.go +++ /dev/null @@ -1,133 +0,0 @@ -package resourcemerge - -import ( - "fmt" - "testing" - - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/equality" -) - -// Test added for now to test ensureContainer (mostly so we don't break Env Vars - proxy) -func TestEnsureContainer(t *testing.T) { - tests := []struct { - existing corev1.Container - input corev1.Container - - expectedModified bool - expected corev1.Container - }{ - // Change in proxy - { - existing: corev1.Container{ - Env: []corev1.EnvVar{ - { - Name: "Proxy", - Value: "Proxy1", - }, - }, - }, - input: corev1.Container{ - Env: []corev1.EnvVar{ - { - Name: "Proxy", - Value: "Proxy2", - }, - }, - }, - expectedModified: true, - expected: corev1.Container{ - Env: []corev1.EnvVar{ - { - Name: "Proxy", - Value: "Proxy2", - }, - }, - }, - }, - // Add Proxy - { - existing: corev1.Container{ - Env: []corev1.EnvVar{}, - }, - input: corev1.Container{ - Env: []corev1.EnvVar{ - { - Name: "Proxy", - Value: "Proxy1", - }, - }, - }, - expectedModified: true, - expected: corev1.Container{ - Env: []corev1.EnvVar{ - { - Name: "Proxy", - Value: "Proxy1", - }, - }, - }, - }, - // Remove Proxy - { - existing: corev1.Container{ - Env: []corev1.EnvVar{ - { - Name: "Proxy", - Value: "Proxy1", - }, - }, - }, - input: corev1.Container{ - Env: []corev1.EnvVar{}, - }, - expectedModified: true, - expected: corev1.Container{ - Env: []corev1.EnvVar{}, - }, - }, - // No change - { - existing: corev1.Container{ - Env: []corev1.EnvVar{ - { - Name: "Proxy", - Value: "Proxy1", - }, - }, - }, - input: corev1.Container{ - Env: []corev1.EnvVar{ - { - Name: "Proxy", - Value: "Proxy1", - }, - }, - }, - expectedModified: false, - expected: corev1.Container{ - Env: []corev1.EnvVar{ - { - Name: "Proxy", - Value: "Proxy1", - }, - }, - }, - }, - } - - for idx, test := range tests { - t.Run(fmt.Sprintf("test#%d", idx), func(t *testing.T) { - modified := BoolPtr(false) - ensureContainer(modified, &test.existing, test.input) - - if *modified != test.expectedModified { - t.Fatalf("mismatch container got: %v want: %v", *modified, test.expectedModified) - } - - if !equality.Semantic.DeepEqual(test.existing, test.expected) { - t.Fatalf("mismatch container got: %v want: %v", test.existing, test.expected) - } - }) - } -} diff --git a/pkg/operator/sync.go b/pkg/operator/sync.go index 1cfc8a8e4a..eac6df728d 100644 --- a/pkg/operator/sync.go +++ b/pkg/operator/sync.go @@ -514,7 +514,14 @@ func (optr *Operator) applyManifests(config *renderConfig, paths manifestPaths) return err } d := resourceread.ReadDaemonSetV1OrDie(dBytes) - _, updated, err := mcoResourceApply.ApplyDaemonSet(optr.kubeClient.AppsV1(), d) + // Pass existing generation until generation can be stored on an MCO CRD + ctx := context.TODO() + client := optr.kubeClient.AppsV1() + existing, err := client.DaemonSets(d.Namespace).Get(ctx, d.Name, metav1.GetOptions{}) + if err != nil { + return err + } + _, updated, err := resourceapply.ApplyDaemonSet(ctx, client, optr.libgoRecorder, d, existing.ObjectMeta.Generation) if err != nil { return err } @@ -553,7 +560,14 @@ func (optr *Operator) syncMachineConfigController(config *renderConfig) error { } mcc := resourceread.ReadDeploymentV1OrDie(mccBytes) - _, updated, err := mcoResourceApply.ApplyDeployment(optr.kubeClient.AppsV1(), mcc) + // Pass existing generation until generation can be stored on an MCO CRD + ctx := context.TODO() + client := optr.kubeClient.AppsV1() + existing, err := client.Deployments(mcc.Namespace).Get(ctx, mcc.Name, metav1.GetOptions{}) + if err != nil { + return err + } + _, updated, err := resourceapply.ApplyDeployment(ctx, client, optr.libgoRecorder, mcc, existing.ObjectMeta.Generation) if err != nil { return err }