From d8ae794003affe9431983074c55d97298d3685ce Mon Sep 17 00:00:00 2001 From: Ivo Petrov Date: Wed, 11 Dec 2024 15:07:24 +0200 Subject: [PATCH] K8s core component validation (#116) * Add core component definitions in release manifest * Align helm chart CRD indentations with kubebuilder generated CRD * Introduce new release manifest CRD changes * make generate * Introduce container comparison logic * Update variable name to improve reusability * Add deployment monitoring permissions to reconciler * Introduce helm release comparison function * Update function to parse K8s distribution * Introduce wait mechanism for K8s core components * Fix typos * Don't fail on job not found errors * Check deployment staus conditions * Fix typos --- api/v1alpha1/releasemanfest_types_test.go | 35 +++ api/v1alpha1/releasemanifest_types.go | 32 +- api/v1alpha1/zz_generated.deepcopy.go | 48 ++- cmd/main.go | 6 +- .../lifecycle.suse.com_releasemanifests.yaml | 58 ++++ config/rbac/role.yaml | 8 + .../templates/release-manifest-crd.yaml | 288 +++++++++++------- internal/controller/helm.go | 13 +- internal/controller/reconcile_kubernetes.go | 118 ++++++- .../controller/reconcile_kubernetes_test.go | 30 +- internal/controller/upgradeplan_controller.go | 1 + internal/upgrade/base.go | 4 +- internal/upgrade/container.go | 41 +++ internal/upgrade/container_test.go | 132 ++++++++ internal/upgrade/helm.go | 2 +- 15 files changed, 663 insertions(+), 153 deletions(-) create mode 100644 internal/upgrade/container.go create mode 100644 internal/upgrade/container_test.go diff --git a/api/v1alpha1/releasemanfest_types_test.go b/api/v1alpha1/releasemanfest_types_test.go index 1efa367..2a8db06 100644 --- a/api/v1alpha1/releasemanfest_types_test.go +++ b/api/v1alpha1/releasemanfest_types_test.go @@ -27,3 +27,38 @@ func TestSupportedArchitectures(t *testing.T) { assert.Contains(t, supported, "amd64") assert.Contains(t, supported, "arm64") } + +func TestConvertContainersSliceToMap(t *testing.T) { + expContainer1 := "foo" + expContainerImage1 := "bar" + expContainer2 := "bar" + expContainerImage2 := "baz" + + component := CoreComponent{ + Containers: []CoreComponentContainer{ + { + Name: expContainer1, + Image: expContainerImage1, + }, + { + Name: expContainer2, + Image: expContainerImage2, + }, + }, + } + + m := component.ConvertContainerSliceToMap() + assert.Equal(t, 2, len(m)) + + v, ok := m[expContainer1] + assert.True(t, ok) + assert.Equal(t, expContainerImage1, v) + + v, ok = m[expContainer2] + assert.True(t, ok) + assert.Equal(t, expContainerImage2, v) + + component = CoreComponent{} + m = component.ConvertContainerSliceToMap() + assert.Empty(t, m) +} diff --git a/api/v1alpha1/releasemanifest_types.go b/api/v1alpha1/releasemanifest_types.go index 8f2bc9b..2235a37 100644 --- a/api/v1alpha1/releasemanifest_types.go +++ b/api/v1alpha1/releasemanifest_types.go @@ -96,7 +96,37 @@ type Kubernetes struct { } type KubernetesDistribution struct { - Version string `json:"version"` + Version string `json:"version"` + CoreComponents []CoreComponent `json:"coreComponents,omitempty"` +} + +// +kubebuilder:validation:Enum=HelmChart;Deployment +type CoreComponentType string + +const ( + HelmChartType CoreComponentType = "HelmChart" + DeploymentType CoreComponentType = "Deployment" +) + +type CoreComponent struct { + Name string `json:"name"` + Version string `json:"version,omitempty"` + Containers []CoreComponentContainer `json:"containers,omitempty"` + Type CoreComponentType `json:"type"` +} + +func (c *CoreComponent) ConvertContainerSliceToMap() map[string]string { + containerMap := map[string]string{} + for _, container := range c.Containers { + containerMap[container.Name] = container.Image + } + + return containerMap +} + +type CoreComponentContainer struct { + Name string `json:"name"` + Image string `json:"image"` } type OperatingSystem struct { diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 61f3746..104d6ca 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -29,7 +29,7 @@ import ( // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Components) DeepCopyInto(out *Components) { *out = *in - out.Kubernetes = in.Kubernetes + in.Kubernetes.DeepCopyInto(&out.Kubernetes) in.OperatingSystem.DeepCopyInto(&out.OperatingSystem) in.Workloads.DeepCopyInto(&out.Workloads) } @@ -44,6 +44,41 @@ func (in *Components) DeepCopy() *Components { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *CoreComponent) DeepCopyInto(out *CoreComponent) { + *out = *in + if in.Containers != nil { + in, out := &in.Containers, &out.Containers + *out = make([]CoreComponentContainer, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CoreComponent. +func (in *CoreComponent) DeepCopy() *CoreComponent { + if in == nil { + return nil + } + out := new(CoreComponent) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *CoreComponentContainer) DeepCopyInto(out *CoreComponentContainer) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CoreComponentContainer. +func (in *CoreComponentContainer) DeepCopy() *CoreComponentContainer { + if in == nil { + return nil + } + out := new(CoreComponentContainer) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *DisableDrain) DeepCopyInto(out *DisableDrain) { *out = *in @@ -116,8 +151,8 @@ func (in *HelmValues) DeepCopy() *HelmValues { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Kubernetes) DeepCopyInto(out *Kubernetes) { *out = *in - out.K3S = in.K3S - out.RKE2 = in.RKE2 + in.K3S.DeepCopyInto(&out.K3S) + in.RKE2.DeepCopyInto(&out.RKE2) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Kubernetes. @@ -133,6 +168,13 @@ func (in *Kubernetes) DeepCopy() *Kubernetes { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *KubernetesDistribution) DeepCopyInto(out *KubernetesDistribution) { *out = *in + if in.CoreComponents != nil { + in, out := &in.CoreComponents, &out.CoreComponents + *out = make([]CoreComponent, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new KubernetesDistribution. diff --git a/cmd/main.go b/cmd/main.go index 0b135a0..bf47530 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -128,9 +128,9 @@ func main() { var watchNamespaces map[string]cache.Config if watchNamespace != "" { watchNamespaces = map[string]cache.Config{ - watchNamespace: {}, - upgrade.HelmChartNamespace: {}, - upgrade.SUCNamespace: {}, + watchNamespace: {}, + upgrade.KubeSystemNamespace: {}, + upgrade.SUCNamespace: {}, } } diff --git a/config/crd/bases/lifecycle.suse.com_releasemanifests.yaml b/config/crd/bases/lifecycle.suse.com_releasemanifests.yaml index 2f80867..68f0e1e 100644 --- a/config/crd/bases/lifecycle.suse.com_releasemanifests.yaml +++ b/config/crd/bases/lifecycle.suse.com_releasemanifests.yaml @@ -45,6 +45,35 @@ spec: properties: k3s: properties: + coreComponents: + items: + properties: + containers: + items: + properties: + image: + type: string + name: + type: string + required: + - image + - name + type: object + type: array + name: + type: string + type: + enum: + - HelmChart + - Deployment + type: string + version: + type: string + required: + - name + - type + type: object + type: array version: type: string required: @@ -52,6 +81,35 @@ spec: type: object rke2: properties: + coreComponents: + items: + properties: + containers: + items: + properties: + image: + type: string + name: + type: string + required: + - image + - name + type: object + type: array + name: + type: string + type: + enum: + - HelmChart + - Deployment + type: string + version: + type: string + required: + - name + - type + type: object + type: array version: type: string required: diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 8012153..d9b2e4a 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -27,6 +27,14 @@ rules: - customresourcedefinitions verbs: - get +- apiGroups: + - apps + resources: + - deployments + verbs: + - get + - list + - watch - apiGroups: - batch resources: diff --git a/helm/upgrade-controller/charts/lifecycle-crds/templates/release-manifest-crd.yaml b/helm/upgrade-controller/charts/lifecycle-crds/templates/release-manifest-crd.yaml index 06d416f..61fa507 100644 --- a/helm/upgrade-controller/charts/lifecycle-crds/templates/release-manifest-crd.yaml +++ b/helm/upgrade-controller/charts/lifecycle-crds/templates/release-manifest-crd.yaml @@ -12,124 +12,182 @@ spec: singular: releasemanifest scope: Namespaced versions: - - name: v1alpha1 - schema: - openAPIV3Schema: - description: ReleaseManifest is the Schema for the releasemanifests API - properties: - apiVersion: - description: |- - APIVersion defines the versioned schema of this representation of an object. - Servers should convert recognized schemas to the latest internal value, and - may reject unrecognized values. - More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources - type: string - kind: - description: |- - Kind is a string value representing the REST resource this object represents. - Servers may infer this from the endpoint the client submits requests to. - Cannot be updated. - In CamelCase. - More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds - type: string - metadata: - type: object - spec: - description: ReleaseManifestSpec defines the desired state of ReleaseManifest - properties: - components: - properties: - kubernetes: - properties: - k3s: + - name: v1alpha1 + schema: + openAPIV3Schema: + description: ReleaseManifest is the Schema for the releasemanifests API + properties: + apiVersion: + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources + type: string + kind: + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + type: string + metadata: + type: object + spec: + description: ReleaseManifestSpec defines the desired state of ReleaseManifest + properties: + components: + properties: + kubernetes: + properties: + k3s: + properties: + coreComponents: + items: + properties: + containers: + items: + properties: + image: + type: string + name: + type: string + required: + - image + - name + type: object + type: array + name: + type: string + type: + enum: + - HelmChart + - Deployment + type: string + version: + type: string + required: + - name + - type + type: object + type: array + version: + type: string + required: + - version + type: object + rke2: + properties: + coreComponents: + items: + properties: + containers: + items: + properties: + image: + type: string + name: + type: string + required: + - image + - name + type: object + type: array + name: + type: string + type: + enum: + - HelmChart + - Deployment + type: string + version: + type: string + required: + - name + - type + type: object + type: array + version: + type: string + required: + - version + type: object + required: + - k3s + - rke2 + type: object + operatingSystem: + properties: + cpeScheme: + type: string + prettyName: + type: string + supportedArchs: + items: + enum: + - x86_64 + - aarch64 + type: string + minItems: 1 + type: array + version: + type: string + zypperID: + type: string + required: + - cpeScheme + - prettyName + - supportedArchs + - version + - zypperID + type: object + workloads: + properties: + helm: + items: properties: - version: + addonCharts: + x-kubernetes-preserve-unknown-fields: true + chart: type: string - required: - - version - type: object - rke2: - properties: + dependencyCharts: + x-kubernetes-preserve-unknown-fields: true + prettyName: + type: string + releaseName: + type: string + repository: + type: string + values: + x-kubernetes-preserve-unknown-fields: true version: type: string required: - - version + - chart + - prettyName + - releaseName + - version type: object - required: - - k3s - - rke2 - type: object - operatingSystem: - properties: - cpeScheme: - type: string - prettyName: - type: string - supportedArchs: - items: - enum: - - x86_64 - - aarch64 - type: string - minItems: 1 - type: array - version: - type: string - zypperID: - type: string - required: - - cpeScheme - - prettyName - - supportedArchs - - version - - zypperID - type: object - workloads: - properties: - helm: - items: - properties: - addonCharts: - x-kubernetes-preserve-unknown-fields: true - chart: - type: string - dependencyCharts: - x-kubernetes-preserve-unknown-fields: true - prettyName: - type: string - releaseName: - type: string - repository: - type: string - values: - x-kubernetes-preserve-unknown-fields: true - version: - type: string - required: - - chart - - prettyName - - releaseName - - version - type: object - type: array - required: - - helm - type: object - required: - - kubernetes - - operatingSystem - - workloads - type: object - releaseVersion: - type: string - required: - - releaseVersion - type: object - status: - description: ReleaseManifestStatus defines the observed state of ReleaseManifest - type: object - type: object - served: true - storage: true - subresources: - status: {} + type: array + required: + - helm + type: object + required: + - kubernetes + - operatingSystem + - workloads + type: object + releaseVersion: + type: string + required: + - releaseVersion + type: object + status: + description: ReleaseManifestStatus defines the observed state of ReleaseManifest + type: object + type: object + served: true + storage: true + subresources: + status: {} diff --git a/internal/controller/helm.go b/internal/controller/helm.go index aee0cc0..6114697 100644 --- a/internal/controller/helm.go +++ b/internal/controller/helm.go @@ -69,6 +69,15 @@ func retrieveHelmRelease(name string) (*helmrelease.Release, error) { return helmRelease, nil } +func compareChartReleaseWithVersion(releaseName string, version string) (bool, error) { + helmRelease, err := retrieveHelmRelease(releaseName) + if err != nil { + return false, fmt.Errorf("retrieving helm release: %w", err) + } + + return helmRelease.Chart.Metadata.Version == version, nil +} + // Updates an existing HelmChart resource in order to trigger an upgrade. func (r *UpgradePlanReconciler) updateHelmChart(ctx context.Context, upgradePlan *lifecyclev1alpha1.UpgradePlan, chart *helmcattlev1.HelmChart, releaseChart *lifecyclev1alpha1.HelmChart) error { backoffLimit := int32(6) @@ -137,7 +146,7 @@ func (r *UpgradePlanReconciler) createHelmChart(ctx context.Context, upgradePlan }, ObjectMeta: metav1.ObjectMeta{ Name: installedChart.Name, - Namespace: upgrade.HelmChartNamespace, + Namespace: upgrade.KubeSystemNamespace, Labels: labels, Annotations: annotations, }, @@ -256,7 +265,7 @@ func (r *UpgradePlanReconciler) upgradeHelmChart(ctx context.Context, upgradePla } job := &batchv1.Job{} - if err = r.Get(ctx, types.NamespacedName{Name: chart.Status.JobName, Namespace: upgrade.HelmChartNamespace}, job); err != nil { + if err = r.Get(ctx, types.NamespacedName{Name: chart.Status.JobName, Namespace: upgrade.KubeSystemNamespace}, job); err != nil { return upgrade.ChartStateUnknown, client.IgnoreNotFound(err) } diff --git a/internal/controller/reconcile_kubernetes.go b/internal/controller/reconcile_kubernetes.go index 65eaab1..ec8f05a 100644 --- a/internal/controller/reconcile_kubernetes.go +++ b/internal/controller/reconcile_kubernetes.go @@ -3,15 +3,20 @@ package controller import ( "context" "fmt" + "slices" "strings" "time" + helmcattlev1 "github.com/k3s-io/helm-controller/pkg/apis/helm.cattle.io/v1" lifecyclev1alpha1 "github.com/suse-edge/upgrade-controller/api/v1alpha1" "github.com/suse-edge/upgrade-controller/internal/upgrade" + appsv1 "k8s.io/api/apps/v1" + batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -24,16 +29,16 @@ func (r *UpgradePlanReconciler) reconcileKubernetes( ) (ctrl.Result, error) { nameSuffix := upgradePlan.Status.SUCNameSuffix - kubernetesVersion, err := targetKubernetesVersion(nodeList, kubernetes) + k8sDistro, err := targetKubernetesDistribution(nodeList, kubernetes) if err != nil { - return ctrl.Result{}, fmt.Errorf("identifying target kubernetes version: %w", err) + return ctrl.Result{}, fmt.Errorf("identifying target kubernetes distribution: %w", err) } conditionType := lifecyclev1alpha1.KubernetesUpgradedCondition identifierLabels := upgrade.PlanIdentifierLabels(upgradePlan.Name, upgradePlan.Namespace) drainControlPlane, drainWorker := parseDrainOptions(nodeList, upgradePlan) - controlPlanePlan := upgrade.KubernetesControlPlanePlan(nameSuffix, kubernetesVersion, drainControlPlane, identifierLabels) + controlPlanePlan := upgrade.KubernetesControlPlanePlan(nameSuffix, k8sDistro.Version, drainControlPlane, identifierLabels) if err = r.Get(ctx, client.ObjectKeyFromObject(controlPlanePlan), controlPlanePlan); err != nil { if !errors.IsNotFound(err) { return ctrl.Result{}, err @@ -48,15 +53,26 @@ func (r *UpgradePlanReconciler) reconcileKubernetes( return ctrl.Result{}, err } - if !isKubernetesUpgraded(nodes, kubernetesVersion) { + if !isKubernetesUpgraded(nodes, k8sDistro.Version) { setInProgressCondition(upgradePlan, conditionType, "Control plane nodes are being upgraded") return ctrl.Result{RequeueAfter: 1 * time.Minute}, nil } else if controlPlaneOnlyCluster(nodeList) { + allUpgraded, waitingFor, err := r.getK8sCoreComponentsUpgradeStatus(ctx, k8sDistro.CoreComponents) + if err != nil { + return ctrl.Result{}, err + } + + if !allUpgraded { + msg := fmt.Sprintf("Waiting for %s core component to be upgraded", waitingFor) + setInProgressCondition(upgradePlan, conditionType, msg) + return ctrl.Result{RequeueAfter: 1 * time.Minute}, nil + } + setSuccessfulCondition(upgradePlan, conditionType, "All cluster nodes are upgraded") return ctrl.Result{Requeue: true}, nil } - workerPlan := upgrade.KubernetesWorkerPlan(nameSuffix, kubernetesVersion, drainWorker, identifierLabels) + workerPlan := upgrade.KubernetesWorkerPlan(nameSuffix, k8sDistro.Version, drainWorker, identifierLabels) if err = r.Get(ctx, client.ObjectKeyFromObject(workerPlan), workerPlan); err != nil { if !errors.IsNotFound(err) { return ctrl.Result{}, err @@ -71,29 +87,109 @@ func (r *UpgradePlanReconciler) reconcileKubernetes( return ctrl.Result{}, err } - if !isKubernetesUpgraded(nodes, kubernetesVersion) { + if !isKubernetesUpgraded(nodes, k8sDistro.Version) { setInProgressCondition(upgradePlan, conditionType, "Worker nodes are being upgraded") return ctrl.Result{RequeueAfter: 1 * time.Minute}, nil } + allUpgraded, waitingFor, err := r.getK8sCoreComponentsUpgradeStatus(ctx, k8sDistro.CoreComponents) + if err != nil { + return ctrl.Result{}, err + } + + if !allUpgraded { + msg := fmt.Sprintf("Waiting for %s core component to be upgraded", waitingFor) + setInProgressCondition(upgradePlan, conditionType, msg) + return ctrl.Result{RequeueAfter: 1 * time.Minute}, nil + } + setSuccessfulCondition(upgradePlan, conditionType, "All cluster nodes are upgraded") return ctrl.Result{Requeue: true}, nil } -func targetKubernetesVersion(nodeList *corev1.NodeList, kubernetes *lifecyclev1alpha1.Kubernetes) (string, error) { +func (r *UpgradePlanReconciler) getK8sCoreComponentsUpgradeStatus(ctx context.Context, core []lifecyclev1alpha1.CoreComponent) (allUpgraded bool, waitingFor string, err error) { + for _, component := range core { + upgraded, err := r.isK8sCoreComponentUpgraded(ctx, &component) + if err != nil { + if errors.IsNotFound(err) { + // Not every component from the core list might be + // on every cluster. + continue + } + return false, "", fmt.Errorf("validating upgrade for component %s: %w", component.Name, err) + } + + if !upgraded { + return false, component.Name, nil + } + } + + return true, "", nil +} + +func (r *UpgradePlanReconciler) isK8sCoreComponentUpgraded(ctx context.Context, component *lifecyclev1alpha1.CoreComponent) (bool, error) { + switch component.Type { + case lifecyclev1alpha1.HelmChartType: + chart := &helmcattlev1.HelmChart{} + if err := r.Get(ctx, upgrade.ChartNamespacedName(component.Name), chart); err != nil { + return false, fmt.Errorf("getting %s helm chart: %w", component.Name, err) + } + + chartJob := &batchv1.Job{} + if err := r.Get(ctx, types.NamespacedName{Name: chart.Status.JobName, Namespace: chart.Namespace}, chartJob); err != nil { + // If the HelmChart exists it must have a Job attached to it. + // If the Job is missing, this indicates a recreate of the Job has been issued + // which can take some time on slower machines. + return false, client.IgnoreNotFound(err) + } + + isJobComplete := func(conditions []batchv1.JobCondition) bool { + return slices.ContainsFunc(conditions, func(condition batchv1.JobCondition) bool { + return condition.Status == corev1.ConditionTrue && condition.Type == batchv1.JobComplete + }) + } + + if !isJobComplete(chartJob.Status.Conditions) { + return false, nil + } + + return compareChartReleaseWithVersion(chart.Name, component.Version) + case lifecyclev1alpha1.DeploymentType: + dep := &appsv1.Deployment{} + if err := r.Get(ctx, types.NamespacedName{Name: component.Name, Namespace: upgrade.KubeSystemNamespace}, dep); err != nil { + return false, fmt.Errorf("getting %s deployment: %w", component.Name, err) + } + + isDeploymentAvailable := func(conditions []appsv1.DeploymentCondition) bool { + return slices.ContainsFunc(conditions, func(condition appsv1.DeploymentCondition) bool { + return condition.Status == corev1.ConditionTrue && condition.Type == appsv1.DeploymentAvailable + }) + } + + if !isDeploymentAvailable(dep.Status.Conditions) { + return false, nil + } + + return upgrade.ContainsContainerImages(dep.Spec.Template.Spec.Containers, component.ConvertContainerSliceToMap(), false), nil + default: + return false, fmt.Errorf("unsupported component type: %s", component.Type) + } +} + +func targetKubernetesDistribution(nodeList *corev1.NodeList, kubernetes *lifecyclev1alpha1.Kubernetes) (*lifecyclev1alpha1.KubernetesDistribution, error) { if len(nodeList.Items) == 0 { - return "", fmt.Errorf("unable to determine current kubernetes version due to empty node list") + return nil, fmt.Errorf("unable to determine current kubernetes distribution due to empty node list") } kubeletVersion := nodeList.Items[0].Status.NodeInfo.KubeletVersion switch { case strings.Contains(kubeletVersion, "k3s"): - return kubernetes.K3S.Version, nil + return &kubernetes.K3S, nil case strings.Contains(kubeletVersion, "rke2"): - return kubernetes.RKE2.Version, nil + return &kubernetes.RKE2, nil default: - return "", fmt.Errorf("upgrading from kubernetes version %s is not supported", kubeletVersion) + return nil, fmt.Errorf("unsupported kubernetes distribution detected in version %s", kubeletVersion) } } diff --git a/internal/controller/reconcile_kubernetes_test.go b/internal/controller/reconcile_kubernetes_test.go index 518ec76..74b3488 100644 --- a/internal/controller/reconcile_kubernetes_test.go +++ b/internal/controller/reconcile_kubernetes_test.go @@ -212,7 +212,7 @@ func TestControlPlaneOnlyCluster(t *testing.T) { })) } -func TestTargetKubernetesVersion(t *testing.T) { +func TestTargetKubernetesDistribution(t *testing.T) { kubernetes := &lifecyclev1alpha1.Kubernetes{ K3S: lifecyclev1alpha1.KubernetesDistribution{ Version: "v1.30.3+k3s1", @@ -223,49 +223,49 @@ func TestTargetKubernetesVersion(t *testing.T) { } tests := []struct { - name string - nodes *corev1.NodeList - expectedVersion string - expectedError string + name string + nodes *corev1.NodeList + expectedDistribution *lifecyclev1alpha1.KubernetesDistribution + expectedError string }{ { name: "Empty node list", nodes: &corev1.NodeList{}, - expectedError: "unable to determine current kubernetes version due to empty node list", + expectedError: "unable to determine current kubernetes distribution due to empty node list", }, { - name: "Unsupported Kubernetes version", + name: "Unsupported Kubernetes distribution", nodes: &corev1.NodeList{ Items: []corev1.Node{{Status: corev1.NodeStatus{NodeInfo: corev1.NodeSystemInfo{KubeletVersion: "v1.30.3"}}}}, }, - expectedError: "upgrading from kubernetes version v1.30.3 is not supported", + expectedError: "unsupported kubernetes distribution detected in version v1.30.3", }, { - name: "Target k3s version", + name: "Target k3s distribution", nodes: &corev1.NodeList{ Items: []corev1.Node{{Status: corev1.NodeStatus{NodeInfo: corev1.NodeSystemInfo{KubeletVersion: "v1.28.12+k3s1"}}}}, }, - expectedVersion: "v1.30.3+k3s1", + expectedDistribution: &kubernetes.K3S, }, { - name: "Target RKE2 version", + name: "Target RKE2 distribution", nodes: &corev1.NodeList{ Items: []corev1.Node{{Status: corev1.NodeStatus{NodeInfo: corev1.NodeSystemInfo{KubeletVersion: "v1.28.12+rke2r1"}}}}, }, - expectedVersion: "v1.30.3+rke2r1", + expectedDistribution: &kubernetes.RKE2, }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - version, err := targetKubernetesVersion(test.nodes, kubernetes) + k8sDistro, err := targetKubernetesDistribution(test.nodes, kubernetes) if test.expectedError != "" { require.Error(t, err) assert.EqualError(t, err, test.expectedError) - assert.Empty(t, version) + assert.Nil(t, k8sDistro) } else { require.NoError(t, err) - assert.Equal(t, test.expectedVersion, version) + assert.Equal(t, test.expectedDistribution, k8sDistro) } }) } diff --git a/internal/controller/upgradeplan_controller.go b/internal/controller/upgradeplan_controller.go index ffb74b3..ea04380 100644 --- a/internal/controller/upgradeplan_controller.go +++ b/internal/controller/upgradeplan_controller.go @@ -64,6 +64,7 @@ type UpgradePlanReconciler struct { // +kubebuilder:rbac:groups=upgrade.cattle.io,resources=plans,verbs=create;list;get;watch;delete // +kubebuilder:rbac:groups="",resources=nodes,verbs=watch;list // +kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;delete;create;watch +// +kubebuilder:rbac:groups=apps,resources=deployments,verbs=get;list;watch // +kubebuilder:rbac:groups=core,resources=events,verbs=create;patch // +kubebuilder:rbac:groups=batch,resources=jobs,verbs=get;list;watch;create // +kubebuilder:rbac:groups=batch,resources=jobs/status,verbs=get diff --git a/internal/upgrade/base.go b/internal/upgrade/base.go index 1266998..74c028d 100644 --- a/internal/upgrade/base.go +++ b/internal/upgrade/base.go @@ -18,8 +18,8 @@ const ( ControlPlaneLabel = "node-role.kubernetes.io/control-plane" - HelmChartNamespace = "kube-system" - SUCNamespace = "cattle-system" + KubeSystemNamespace = "kube-system" + SUCNamespace = "cattle-system" controlPlaneKey = "control-plane" workersKey = "workers" diff --git a/internal/upgrade/container.go b/internal/upgrade/container.go new file mode 100644 index 0000000..6060a30 --- /dev/null +++ b/internal/upgrade/container.go @@ -0,0 +1,41 @@ +package upgrade + +import ( + "strings" + + corev1 "k8s.io/api/core/v1" +) + +// ContainsContainerImages validates that a given map of "container: image" references +// exist in a slice of corev1.Containers. +// +// Returns 'true' only if all the "container: image" references from the 'contains' map +// are present in the corev1.Containers slice. +// +// If 'strict' is true, will require for the corev1.Container.Image to be exactly the same +// as the image string defined in the 'contains' map. +// +// If 'strict' is false, will require for the corev1.Container.Image to contain the image string defined +// in the 'contains' map. Useful for use-cases where the image registry may change +// based on the environment (e.g. private registry). +func ContainsContainerImages(containers []corev1.Container, contains map[string]string, strict bool) bool { + foundContainers := 0 + for _, container := range containers { + image, ok := contains[container.Name] + if !ok { + // Skip containers that are not in the 'contains' map. + continue + } + foundContainers++ + + if strict && container.Image != image { + return false + } + + if !strict && !strings.Contains(container.Image, image) { + return false + } + } + + return foundContainers == len(contains) +} diff --git a/internal/upgrade/container_test.go b/internal/upgrade/container_test.go new file mode 100644 index 0000000..982110d --- /dev/null +++ b/internal/upgrade/container_test.go @@ -0,0 +1,132 @@ +package upgrade + +import ( + "testing" + + "github.com/stretchr/testify/assert" + corev1 "k8s.io/api/core/v1" +) + +func TestContainsContainerImagesStrict(t *testing.T) { + additionalStrictTest := ContainersImageTest{ + Name: "Container image registry mismatch", + Containers: []corev1.Container{ + { + Name: "coredns", + Image: "foo.bar:8080/rancher/mirrored-coredns-coredns:1.11.3", + }, + }, + ContainerImages: map[string]string{ + "coredns": "rancher/mirrored-coredns-coredns:1.11.3", + }, + ExpectedResult: false, + } + + allTests := append(getDefaultContainerImageTests(), additionalStrictTest) + + for _, test := range allTests { + t.Run(test.Name, func(t *testing.T) { + if test.ExpectedResult { + assert.True(t, ContainsContainerImages(test.Containers, test.ContainerImages, true)) + } else { + assert.False(t, ContainsContainerImages(test.Containers, test.ContainerImages, true)) + } + }) + } +} + +func TestContainsContainerImagesLenient(t *testing.T) { + additionalLenientTest := ContainersImageTest{ + Name: "Container image registry mismatch", + Containers: []corev1.Container{ + { + Name: "coredns", + Image: "foo.bar:8080/rancher/mirrored-coredns-coredns:1.11.3", + }, + }, + ContainerImages: map[string]string{ + "coredns": "rancher/mirrored-coredns-coredns:1.11.3", + }, + ExpectedResult: true, + } + + allTests := append(getDefaultContainerImageTests(), additionalLenientTest) + + for _, test := range allTests { + t.Run(test.Name, func(t *testing.T) { + if test.ExpectedResult { + assert.True(t, ContainsContainerImages(test.Containers, test.ContainerImages, false)) + } else { + assert.False(t, ContainsContainerImages(test.Containers, test.ContainerImages, false)) + } + }) + } +} + +type ContainersImageTest struct { + Name string + Containers []corev1.Container + ContainerImages map[string]string + ExpectedResult bool +} + +func getDefaultContainerImageTests() []ContainersImageTest { + return []ContainersImageTest{ + { + Name: "Missing container image for comparison", + Containers: []corev1.Container{ + { + Name: "foo", + Image: "bar/baz:0.0.0", + }, + }, + ContainerImages: map[string]string{ + "coredns": "rancher/mirrored-coredns-coredns:1.11.3", + }, + ExpectedResult: false, + }, + { + Name: "Container image version mismatch", + Containers: []corev1.Container{ + { + Name: "coredns", + Image: "rancher/mirrored-coredns-coredns:1.11.2", + }, + }, + ContainerImages: map[string]string{ + "coredns": "rancher/mirrored-coredns-coredns:1.11.3", + }, + ExpectedResult: false, + }, + { + Name: "Matching container image without additional images", + Containers: []corev1.Container{ + { + Name: "coredns", + Image: "rancher/mirrored-coredns-coredns:1.11.3", + }, + }, + ContainerImages: map[string]string{ + "coredns": "rancher/mirrored-coredns-coredns:1.11.3", + }, + ExpectedResult: true, + }, + { + Name: "Matching container image with additional images", + Containers: []corev1.Container{ + { + Name: "coredns", + Image: "rancher/mirrored-coredns-coredns:1.11.3", + }, + { + Name: "sidecar", + Image: "foo/bar:0.0.0", + }, + }, + ContainerImages: map[string]string{ + "coredns": "rancher/mirrored-coredns-coredns:1.11.3", + }, + ExpectedResult: true, + }, + } +} diff --git a/internal/upgrade/helm.go b/internal/upgrade/helm.go index 3df7fc7..4b37c47 100644 --- a/internal/upgrade/helm.go +++ b/internal/upgrade/helm.go @@ -9,7 +9,7 @@ import ( func ChartNamespacedName(chart string) types.NamespacedName { return types.NamespacedName{ Name: chart, - Namespace: HelmChartNamespace, + Namespace: KubeSystemNamespace, } }