Merge pull request #792 from dprince/ovn_update_order_edpm

Enforce update order for OVN for Ctlplane/EDPM

Author: openshift-merge-bot[bot]

config/rbac/role.yaml

@@ -180,6 +180,14 @@ rules:
   - get
   - patch
   - update
+- apiGroups:
+  - dataplane.openstack.org
+  resources:
+  - openstackdataplanenodesets
+  verbs:
+  - get
+  - list
+  - watch
 - apiGroups:
   - designate.openstack.org
   resources:

controllers/core/openstackcontrolplane_controller.go

@@ -245,7 +245,9 @@ func (r *OpenStackControlPlaneReconciler) Reconcile(ctx context.Context, req ctr
 			}
 			instance.Status.DeployedOVNVersion = &version.Spec.TargetVersion
 			return ctrl.Result{}, nil
-		} else if !version.Status.Conditions.IsTrue(corev1beta1.OpenStackVersionMinorUpdateControlplane) {
+		} else if version.Status.Conditions.IsTrue(corev1beta1.OpenStackVersionMinorUpdateOVNDataplane) &&
+			!version.Status.Conditions.IsTrue(corev1beta1.OpenStackVersionMinorUpdateControlplane) {
+
 			Log.Info("Minor update on the ControlPlane")
 			ctrlResult, err := r.reconcileNormal(ctx, instance, version, helper)
 			if err != nil {

controllers/core/openstackversion_controller.go

@@ -32,6 +32,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	"github.com/go-logr/logr"
+	dataplanev1 "github.com/openstack-k8s-operators/dataplane-operator/api/v1beta1"
 	condition "github.com/openstack-k8s-operators/lib-common/modules/common/condition"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/helper"
 	corev1beta1 "github.com/openstack-k8s-operators/openstack-operator/apis/core/v1beta1"
@@ -56,16 +57,6 @@ func SetupVersionDefaults() {
 	envContainerImages = localVars
 }
 
-func compareStringPointers(a, b *string) bool {
-	if a == nil && b == nil {
-		return true
-	}
-	if a == nil || b == nil {
-		return false
-	}
-	return *a == *b
-}
-
 // OpenStackVersionReconciler reconciles a OpenStackVersion object
 type OpenStackVersionReconciler struct {
 	client.Client
@@ -79,9 +70,11 @@ func (r *OpenStackVersionReconciler) GetLogger(ctx context.Context) logr.Logger
 	return log.FromContext(ctx).WithName("Controllers").WithName("OpenStackVersion")
 }
 
-//+kubebuilder:rbac:groups=core.openstack.org,resources=openstackversions,verbs=get;list;watch;create;update;patch;delete
-//+kubebuilder:rbac:groups=core.openstack.org,resources=openstackversions/status,verbs=get;update;patch
-//+kubebuilder:rbac:groups=core.openstack.org,resources=openstackversions/finalizers,verbs=update
+// +kubebuilder:rbac:groups=core.openstack.org,resources=openstackversions,verbs=get;list;watch;create;update;patch;delete
+// +kubebuilder:rbac:groups=core.openstack.org,resources=openstackversions/status,verbs=get;update;patch
+// +kubebuilder:rbac:groups=core.openstack.org,resources=openstackversions/finalizers,verbs=update
+// +kubebuilder:rbac:groups=core.openstack.org,resources=openstackcontrolplanes,verbs=get;list;watch
+// +kubebuilder:rbac:groups=dataplane.openstack.org,resources=openstackdataplanenodesets,verbs=get;list;watch
 
 // Reconcile is part of the main kubernetes reconciliation loop which aims to
 // move the current state of the cluster closer to the desired state.
@@ -159,10 +152,10 @@ func (r *OpenStackVersionReconciler) Reconcile(ctx context.Context, req ctrl.Req
 	// no minor update conditions unless we have a deployed version
 	if instance.Status.DeployedVersion != nil && instance.Spec.TargetVersion != *instance.Status.DeployedVersion {
 		cl = append(cl, *condition.UnknownCondition(corev1beta1.OpenStackVersionMinorUpdateOVNControlplane, condition.InitReason, string(corev1beta1.OpenStackVersionMinorUpdateInitMessage)),
-			*condition.UnknownCondition(corev1beta1.OpenStackVersionMinorUpdateControlplane, condition.InitReason, string(corev1beta1.OpenStackVersionMinorUpdateInitMessage)))
-		// fixme add dataplane conditions here
-		//condition.UnknownCondition(corev1beta1.OpenStackVersionMinorUpdateOVNDataplane, condition.InitReason, string(corev1beta1.OpenStackVersionMinorUpdateInitMessage)),
-		//condition.UnknownCondition(corev1beta1.OpenStackVersionMinorUpdateDataplane, condition.InitReason, string(corev1beta1.OpenStackVersionMinorUpdateInitMessage)))
+			*condition.UnknownCondition(corev1beta1.OpenStackVersionMinorUpdateOVNDataplane, condition.InitReason, string(corev1beta1.OpenStackVersionMinorUpdateInitMessage)),
+			*condition.UnknownCondition(corev1beta1.OpenStackVersionMinorUpdateControlplane, condition.InitReason, string(corev1beta1.OpenStackVersionMinorUpdateInitMessage)),
+			*condition.UnknownCondition(corev1beta1.OpenStackVersionMinorUpdateDataplane, condition.InitReason, string(corev1beta1.OpenStackVersionMinorUpdateInitMessage)),
+		)
 	}
 	instance.Status.Conditions.Init(&cl)
 	instance.Status.ObservedGeneration = instance.Generation
@@ -182,13 +175,14 @@ func (r *OpenStackVersionReconciler) Reconcile(ctx context.Context, req ctrl.Req
 
 	instance.Status.AvailableVersion = &envAvailableVersion
 	defaults := openstack.InitializeOpenStackVersionImageDefaults(ctx, envContainerImages)
-	// store the defaults for the currently available version
 	if instance.Status.ContainerImageVersionDefaults == nil {
 		instance.Status.ContainerImageVersionDefaults = make(map[string]*corev1beta1.ContainerDefaults)
 	}
+	// store the defaults for the currently available version
 	instance.Status.ContainerImageVersionDefaults[envAvailableVersion] = defaults
 
 	// calculate the container images for the target version
+	Log.Info("Target version: ", "targetVersion", instance.Spec.TargetVersion)
 	val, ok := instance.Status.ContainerImageVersionDefaults[instance.Spec.TargetVersion]
 	if !ok {
 		Log.Info("Target version not found in defaults", "targetVersion", instance.Spec.TargetVersion)
@@ -208,25 +202,30 @@ func (r *OpenStackVersionReconciler) Reconcile(ctx context.Context, req ctrl.Req
 		Name:      instance.Name,
 	}, controlPlane)
 	if err != nil {
-		// we ignore not found
 		if k8s_errors.IsNotFound(err) {
-			Log.Info("No controlplane found.")
+			Log.Info("Controlplane not found:", "instance name", instance.Name)
 			return ctrl.Result{}, nil
 		}
 		return ctrl.Result{}, err
 	}
 
-	// greenfield deployment //FIXME check dataplane here too
-	if controlPlane.Status.DeployedVersion == nil {
-		Log.Info("No controlplane or controlplane is not deployed")
-		return ctrl.Result{}, nil
+	// lookup nodesets
+	dataplaneNodesets, err := openstack.GetDataplaneNodesets(ctx, controlPlane, versionHelper)
+	if err != nil {
+		Log.Error(err, "Failed to get dataplane nodesets")
+		return ctrl.Result{}, err
 	}
 
-	// TODO minor update for OVN Dataplane in progress
+	// greenfield deployment
+	if controlPlane.Status.DeployedVersion == nil && !openstack.DataplaneNodesetsDeployedVersionIsSet(dataplaneNodesets) {
+		Log.Info("Waiting for controlplane and dataplane nodesets to be deployed.")
+		return ctrl.Result{}, nil
+	}
 
-	// minor update for OVN Controlplane in progress
+	// minor update in progress
 	if instance.Status.DeployedVersion != nil && instance.Spec.TargetVersion != *instance.Status.DeployedVersion {
-		if !compareStringPointers(controlPlane.Status.ContainerImages.OvnControllerImage, instance.Status.ContainerImages.OvnControllerImage) ||
+
+		if !openstack.OVNControllerImageMatch(controlPlane, instance) ||
 			!controlPlane.Status.Conditions.IsTrue(corev1beta1.OpenStackControlPlaneOVNReadyCondition) {
 			instance.Status.Conditions.Set(condition.FalseCondition(
 				corev1beta1.OpenStackVersionMinorUpdateOVNControlplane,
@@ -240,10 +239,22 @@ func (r *OpenStackVersionReconciler) Reconcile(ctx context.Context, req ctrl.Req
 			corev1beta1.OpenStackVersionMinorUpdateOVNControlplane,
 			corev1beta1.OpenStackVersionMinorUpdateReadyMessage)
 
+		// minor update for Dataplane OVN
+		if !openstack.DataplaneNodesetsOVNControllerImagesMatch(instance, dataplaneNodesets) {
+			instance.Status.Conditions.Set(condition.FalseCondition(
+				corev1beta1.OpenStackVersionMinorUpdateOVNDataplane,
+				condition.RequestedReason,
+				condition.SeverityInfo,
+				corev1beta1.OpenStackVersionMinorUpdateReadyRunningMessage))
+			Log.Info("Waiting on OVN Dataplane updates to complete")
+			return ctrl.Result{}, nil
+		}
+		instance.Status.Conditions.MarkTrue(
+			corev1beta1.OpenStackVersionMinorUpdateOVNDataplane,
+			corev1beta1.OpenStackVersionMinorUpdateReadyMessage)
+
 		// minor update for Controlplane in progress
-		// we only check keystone here as it will only get updated during this phase
-		// FIXME: add checks to all images on the Controlplane here once conditions and observedGeneration work are finished
-		if !compareStringPointers(controlPlane.Status.ContainerImages.KeystoneAPIImage, instance.Status.ContainerImages.KeystoneAPIImage) ||
+		if !openstack.ControlplaneContainerImageMatch(controlPlane, instance) ||
 			!controlPlane.IsReady() {
 			instance.Status.Conditions.Set(condition.FalseCondition(
 				corev1beta1.OpenStackVersionMinorUpdateControlplane,
@@ -253,11 +264,23 @@ func (r *OpenStackVersionReconciler) Reconcile(ctx context.Context, req ctrl.Req
 			Log.Info("Minor update for Controlplane in progress")
 			return ctrl.Result{}, nil
 		}
-		// TODO minor update for Dataplane in progress goes here
-
 		instance.Status.Conditions.MarkTrue(
 			corev1beta1.OpenStackVersionMinorUpdateControlplane,
 			corev1beta1.OpenStackVersionMinorUpdateReadyMessage)
+
+		if !openstack.DataplaneNodesetsDeployed(instance, dataplaneNodesets) {
+			instance.Status.Conditions.Set(condition.FalseCondition(
+				corev1beta1.OpenStackVersionMinorUpdateDataplane,
+				condition.RequestedReason,
+				condition.SeverityInfo,
+				corev1beta1.OpenStackVersionMinorUpdateReadyRunningMessage))
+			Log.Info("Waiting on Dataplane update to complete")
+			return ctrl.Result{}, nil
+		}
+
+		instance.Status.Conditions.MarkTrue(
+			corev1beta1.OpenStackVersionMinorUpdateDataplane,
+			corev1beta1.OpenStackVersionMinorUpdateReadyMessage)
 	}
 
 	if controlPlane.IsReady() {
@@ -301,6 +324,7 @@ func (r *OpenStackVersionReconciler) SetupWithManager(mgr ctrl.Manager) error {
 
 	return ctrl.NewControllerManagedBy(mgr).
 		Watches(&corev1beta1.OpenStackControlPlane{}, versionFunc).
+		Watches(&dataplanev1.OpenStackDataPlaneNodeSet{}, versionFunc).
 		For(&corev1beta1.OpenStackVersion{}).
 		Complete(r)
 }

pkg/openstack/barbican.go

@@ -33,6 +33,9 @@ func ReconcileBarbican(ctx context.Context, instance *corev1beta1.OpenStackContr
 		}
 		instance.Status.Conditions.Remove(corev1beta1.OpenStackControlPlaneBarbicanReadyCondition)
 		instance.Status.Conditions.Remove(corev1beta1.OpenStackControlPlaneExposeBarbicanReadyCondition)
+		instance.Status.ContainerImages.BarbicanAPIImage = nil
+		instance.Status.ContainerImages.BarbicanWorkerImage = nil
+		instance.Status.ContainerImages.BarbicanKeystoneListenerImage = nil
 		return ctrl.Result{}, nil
 	}
 
@@ -155,3 +158,17 @@ func ReconcileBarbican(ctx context.Context, instance *corev1beta1.OpenStackContr
 
 	return ctrl.Result{}, nil
 }
+
+// BarbicanImageMatch - return true if the Barbican images match on the ControlPlane and Version, or if Barbican is not enabled
+func BarbicanImageMatch(controlPlane *corev1beta1.OpenStackControlPlane, version *corev1beta1.OpenStackVersion) bool {
+
+	if controlPlane.Spec.Barbican.Enabled {
+		if !stringPointersEqual(controlPlane.Status.ContainerImages.BarbicanAPIImage, version.Status.ContainerImages.BarbicanAPIImage) ||
+			!stringPointersEqual(controlPlane.Status.ContainerImages.BarbicanWorkerImage, version.Status.ContainerImages.BarbicanWorkerImage) ||
+			!stringPointersEqual(controlPlane.Status.ContainerImages.BarbicanKeystoneListenerImage, version.Status.ContainerImages.BarbicanKeystoneListenerImage) {
+			return false
+		}
+	}
+
+	return true
+}

pkg/openstack/cinder.go

@@ -34,6 +34,10 @@ func ReconcileCinder(ctx context.Context, instance *corev1beta1.OpenStackControl
 		}
 		instance.Status.Conditions.Remove(corev1beta1.OpenStackControlPlaneCinderReadyCondition)
 		instance.Status.Conditions.Remove(corev1beta1.OpenStackControlPlaneExposeCinderReadyCondition)
+		instance.Status.ContainerImages.CinderAPIImage = nil
+		instance.Status.ContainerImages.CinderSchedulerImage = nil
+		instance.Status.ContainerImages.CinderBackupImage = nil
+		instance.Status.ContainerImages.CinderVolumeImages = make(map[string]*string)
 		return ctrl.Result{}, nil
 	}
 	Log := GetLogger(ctx)
@@ -183,3 +187,22 @@ func ReconcileCinder(ctx context.Context, instance *corev1beta1.OpenStackControl
 	return ctrl.Result{}, nil
 
 }
+
+// CinderImageMatch - return true if the Cinder images match on the ControlPlane and Version, or if Cinder is not enabled
+func CinderImageMatch(controlPlane *corev1beta1.OpenStackControlPlane, version *corev1beta1.OpenStackVersion) bool {
+
+	if controlPlane.Spec.Cinder.Enabled {
+		if !stringPointersEqual(controlPlane.Status.ContainerImages.CinderAPIImage, version.Status.ContainerImages.CinderAPIImage) ||
+			!stringPointersEqual(controlPlane.Status.ContainerImages.CinderSchedulerImage, version.Status.ContainerImages.CinderSchedulerImage) ||
+			!stringPointersEqual(controlPlane.Status.ContainerImages.CinderBackupImage, version.Status.ContainerImages.CinderBackupImage) {
+			return false
+		}
+		for name, img := range version.Status.ContainerImages.CinderVolumeImages {
+			if !stringPointersEqual(controlPlane.Status.ContainerImages.CinderVolumeImages[name], img) {
+				return false
+			}
+		}
+	}
+
+	return true
+}

pkg/openstack/dataplane.go

@@ -0,0 +1,71 @@
+package openstack
+
+import (
+	"context"
+
+	"github.com/openstack-k8s-operators/lib-common/modules/common/helper"
+	corev1beta1 "github.com/openstack-k8s-operators/openstack-operator/apis/core/v1beta1"
+
+	dataplanev1 "github.com/openstack-k8s-operators/dataplane-operator/api/v1beta1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+// GetDataplaneNodesets - returns the dataplanenodesets in the namespace of the controlplane
+func GetDataplaneNodesets(ctx context.Context, instance *corev1beta1.OpenStackControlPlane, helper *helper.Helper) (*dataplanev1.OpenStackDataPlaneNodeSetList, error) {
+	// Get the dataplane nodesets
+	dataplaneNodesets := &dataplanev1.OpenStackDataPlaneNodeSetList{}
+	opts := []client.ListOption{
+		client.InNamespace(instance.Namespace),
+	}
+	err := helper.GetClient().List(ctx, dataplaneNodesets, opts...)
+	if err != nil {
+		return nil, err
+	}
+	return dataplaneNodesets, nil
+}
+
+func DataplaneNodesetsDeployedVersionIsSet(dataplaneNodesets *dataplanev1.OpenStackDataPlaneNodeSetList) bool {
+	for _, nodeset := range dataplaneNodesets.Items {
+		// FIXME: DeployedVersion on the DataplaneNodeset should be a string pointer to match how Controlplane implements this
+		if nodeset.Status.DeployedVersion == "" {
+			return false
+		}
+	}
+	return true
+}
+
+// DataplaneNodesetsOVNControllerImagesMatch returns true if OVNControllers are deployed on all nodesets
+func DataplaneNodesetsOVNControllerImagesMatch(version *corev1beta1.OpenStackVersion, dataplaneNodesets *dataplanev1.OpenStackDataPlaneNodeSetList) bool {
+	for _, nodeset := range dataplaneNodesets.Items {
+		if nodeset.Generation != nodeset.Status.ObservedGeneration {
+			return false
+		}
+		// we only check nodesets if they deploy OVN
+		if nodeset.Status.ContainerImages["OvnControllerImage"] != "" {
+			if !nodeset.IsReady() {
+				return false
+			}
+			if nodeset.Status.ContainerImages["OvnControllerImage"] != *version.Status.ContainerImages.OvnControllerImage {
+				return false
+			}
+		}
+	}
+	return true
+}
+
+// DataplaneNodesetsDeployed returns true if all nodesets are deployed with the latest version
+func DataplaneNodesetsDeployed(version *corev1beta1.OpenStackVersion, dataplaneNodesets *dataplanev1.OpenStackDataPlaneNodeSetList) bool {
+	for _, nodeset := range dataplaneNodesets.Items {
+		if nodeset.Generation != nodeset.Status.ObservedGeneration {
+			return false
+		}
+		if !nodeset.IsReady() {
+			return false
+		}
+		if nodeset.Status.DeployedVersion != version.Spec.TargetVersion {
+			return false
+		}
+
+	}
+	return true
+}

pkg/openstack/designate.go

@@ -34,6 +34,13 @@ func ReconcileDesignate(ctx context.Context, instance *corev1beta1.OpenStackCont
 		}
 		instance.Status.Conditions.Remove(corev1beta1.OpenStackControlPlaneDesignateReadyCondition)
 		instance.Status.Conditions.Remove(corev1beta1.OpenStackControlPlaneExposeDesignateReadyCondition)
+		instance.Status.ContainerImages.DesignateAPIImage = nil
+		instance.Status.ContainerImages.DesignateCentralImage = nil
+		instance.Status.ContainerImages.DesignateMdnsImage = nil
+		instance.Status.ContainerImages.DesignateProducerImage = nil
+		instance.Status.ContainerImages.DesignateWorkerImage = nil
+		instance.Status.ContainerImages.DesignateBackendbind9Image = nil
+		instance.Status.ContainerImages.DesignateUnboundImage = nil
 		return ctrl.Result{}, nil
 	}
 
@@ -169,3 +176,21 @@ func ReconcileDesignate(ctx context.Context, instance *corev1beta1.OpenStackCont
 	return ctrl.Result{}, nil
 
 }
+
+// DesignateImageMatch - return true if the Designate images match on the ControlPlane and Version, or if Designate is not enabled
+func DesignateImageMatch(controlPlane *corev1beta1.OpenStackControlPlane, version *corev1beta1.OpenStackVersion) bool {
+
+	if controlPlane.Spec.Designate.Enabled {
+		if !stringPointersEqual(controlPlane.Status.ContainerImages.DesignateAPIImage, version.Status.ContainerImages.DesignateAPIImage) ||
+			!stringPointersEqual(controlPlane.Status.ContainerImages.DesignateCentralImage, version.Status.ContainerImages.DesignateCentralImage) ||
+			!stringPointersEqual(controlPlane.Status.ContainerImages.DesignateMdnsImage, version.Status.ContainerImages.DesignateMdnsImage) ||
+			!stringPointersEqual(controlPlane.Status.ContainerImages.DesignateProducerImage, version.Status.ContainerImages.DesignateProducerImage) ||
+			!stringPointersEqual(controlPlane.Status.ContainerImages.DesignateWorkerImage, version.Status.ContainerImages.DesignateWorkerImage) ||
+			!stringPointersEqual(controlPlane.Status.ContainerImages.DesignateBackendbind9Image, version.Status.ContainerImages.DesignateBackendbind9Image) ||
+			!stringPointersEqual(controlPlane.Status.ContainerImages.DesignateUnboundImage, version.Status.ContainerImages.DesignateUnboundImage) {
+			return false
+		}
+	}
+
+	return true
+}

pkg/openstack/dnsmasq.go

@@ -30,6 +30,7 @@ func ReconcileDNSMasqs(ctx context.Context, instance *corev1beta1.OpenStackContr
 			return res, err
 		}
 		instance.Status.Conditions.Remove(corev1beta1.OpenStackControlPlaneDNSReadyCondition)
+		instance.Status.ContainerImages.InfraDnsmasqImage = nil
 		return ctrl.Result{}, nil
 	}
 
@@ -76,3 +77,14 @@ func ReconcileDNSMasqs(ctx context.Context, instance *corev1beta1.OpenStackContr
 	return ctrl.Result{}, nil
 
 }
+
+// DnsmasqImageMatch - return true if the Dnsmasq images match on the ControlPlane and Version, or if Dnsmasq is not enabled
+func DnsmasqImageMatch(controlPlane *corev1beta1.OpenStackControlPlane, version *corev1beta1.OpenStackVersion) bool {
+
+	if controlPlane.Spec.DNS.Enabled {
+		if !stringPointersEqual(controlPlane.Status.ContainerImages.InfraDnsmasqImage, version.Status.ContainerImages.InfraDnsmasqImage) {
+			return false
+		}
+	}
+	return true
+}