Merge pull request #736 from hidekazuna/update_sync

Update sync logic

Author: k8s-ci-robot

api/v1alpha3/openstackmachine_types.go

@@ -33,6 +33,9 @@ type OpenStackMachineSpec struct {
 	// ProviderID is the unique identifier as specified by the cloud provider.
 	ProviderID *string `json:"providerID,omitempty"`
 
+	// InstanceID is the OpenStack instance ID for this machine.
+	InstanceID *string `json:"instanceID,omitempty"`
+
 	// The name of the secret containing the openstack credentials
 	// +optional
 	CloudsSecret *corev1.SecretReference `json:"cloudsSecret"`

api/v1alpha3/openstackmachine_webhook.go

@@ -72,6 +72,10 @@ func (r *OpenStackMachine) ValidateUpdate(old runtime.Object) error {
 	delete(oldOpenStackMachineSpec, "providerID")
 	delete(newOpenStackMachineSpec, "providerID")
 
+	// allow changes to instanceID
+	delete(oldOpenStackMachineSpec, "instanceID")
+	delete(newOpenStackMachineSpec, "instanceID")
+
 	if !reflect.DeepEqual(oldOpenStackMachineSpec, newOpenStackMachineSpec) {
 		allErrs = append(allErrs, field.Forbidden(field.NewPath("spec"), "cannot be modified"))
 	}

api/v1alpha3/zz_generated.deepcopy.go

@@ -90,6 +90,9 @@ spec:
                   If the RootVolume is specified, this will be ignored and use rootVolume
                   directly.
                 type: string
+              instanceID:
+                description: InstanceID is the OpenStack instance ID for this machine.
+                type: string
               networks:
                 description: A networks object. Required parameter when there are
                   multiple networks defined for the tenant. When you do not specify

config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachines.yaml

@@ -82,6 +82,10 @@ spec:
                           instance. If the RootVolume is specified, this will be ignored
                           and use rootVolume directly.
                         type: string
+                      instanceID:
+                        description: InstanceID is the OpenStack instance ID for this
+                          machine.
+                        type: string
                       networks:
                         description: A networks object. Required parameter when there
                           are multiple networks defined for the tenant. When you do

config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachinetemplates.yaml

@@ -17,32 +17,40 @@ limitations under the License.
 package controllers
 
 import (
+	"strings"
+
+	"github.com/go-logr/logr"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3"
+	"k8s.io/apimachinery/pkg/runtime"
 	clusterutil "sigs.k8s.io/cluster-api/util"
 	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 )
 
-// TODO: Move to Cluster API
-var pausePredicates = predicate.Funcs{
-	UpdateFunc: func(e event.UpdateEvent) bool {
-		return !isPaused(nil, e.MetaNew)
-	},
-	CreateFunc: func(e event.CreateEvent) bool {
-		return !isPaused(nil, e.Meta)
-	},
-	DeleteFunc: func(e event.DeleteEvent) bool {
-		return !isPaused(nil, e.Meta)
-	},
+func pausedPredicates(logger logr.Logger) predicate.Funcs {
+	return predicate.Funcs{
+		UpdateFunc: func(e event.UpdateEvent) bool {
+			return processIfUnpaused(logger.WithValues("predicate", "updateEvent"), e.ObjectNew, e.MetaNew)
+		},
+		CreateFunc: func(e event.CreateEvent) bool {
+			return processIfUnpaused(logger.WithValues("predicate", "createEvent"), e.Object, e.Meta)
+		},
+		DeleteFunc: func(e event.DeleteEvent) bool {
+			return processIfUnpaused(logger.WithValues("predicate", "deleteEvent"), e.Object, e.Meta)
+		},
+		GenericFunc: func(e event.GenericEvent) bool {
+			return processIfUnpaused(logger.WithValues("predicate", "genericEvent"), e.Object, e.Meta)
+		},
+	}
 }
 
-// TODO: Fix up Cluster API's clusterutil.IsPaused function
-func isPaused(cluster *clusterv1.Cluster, v metav1.Object) bool {
-	if cluster == nil {
-		cluster = &clusterv1.Cluster{
-			Spec: clusterv1.ClusterSpec{},
-		}
+func processIfUnpaused(logger logr.Logger, obj runtime.Object, meta metav1.Object) bool {
+	kind := strings.ToLower(obj.GetObjectKind().GroupVersionKind().Kind)
+	log := logger.WithValues("namespace", meta.GetNamespace(), kind, meta.GetName())
+	if clusterutil.HasPausedAnnotation(meta) {
+		log.V(4).Info("Resource is paused, will not attempt to map resource")
+		return false
 	}
-	return clusterutil.IsPaused(cluster, v)
+	log.V(4).Info("Resource is not paused, will attempt to map resource")
+	return true
 }

controllers/helpers.go

@@ -19,6 +19,7 @@ package controllers
 import (
 	"context"
 	"fmt"
+	"reflect"
 
 	"github.com/go-logr/logr"
 	"github.com/gophercloud/gophercloud"
@@ -41,7 +42,11 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
+	"sigs.k8s.io/controller-runtime/pkg/event"
+	"sigs.k8s.io/controller-runtime/pkg/handler"
+	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
+	"sigs.k8s.io/controller-runtime/pkg/source"
 )
 
 // OpenStackClusterReconciler reconciles a OpenStackCluster object
@@ -75,11 +80,6 @@ func (r *OpenStackClusterReconciler) Reconcile(req ctrl.Request) (_ ctrl.Result,
 		return reconcile.Result{}, err
 	}
 
-	if isPaused(cluster, openStackCluster) {
-		log.Info("OpenStackCluster or linked Cluster is marked as paused. Won't reconcile")
-		return reconcile.Result{}, nil
-	}
-
 	if cluster == nil {
 		log.Info("Cluster Controller has not yet set OwnerRef")
 		return reconcile.Result{}, nil
@@ -438,9 +438,109 @@ func (r *OpenStackClusterReconciler) reconcileNetworkComponents(log logr.Logger,
 }
 
 func (r *OpenStackClusterReconciler) SetupWithManager(mgr ctrl.Manager, options controller.Options) error {
-	return ctrl.NewControllerManagedBy(mgr).
+	controller, err := ctrl.NewControllerManagedBy(mgr).
 		WithOptions(options).
 		For(&infrav1.OpenStackCluster{}).
-		WithEventFilter(pausePredicates).
-		Complete(r)
+		WithEventFilter(pausedPredicates(r.Log)).
+		WithEventFilter(
+			predicate.Funcs{
+				// Avoid reconciling if the event triggering the reconciliation is related to incremental status updates
+				// for OpenStackCluster resources only
+				UpdateFunc: func(e event.UpdateEvent) bool {
+					if e.ObjectOld.GetObjectKind().GroupVersionKind().Kind != "OpenStackCluster" {
+						return true
+					}
+
+					oldCluster := e.ObjectOld.(*infrav1.OpenStackCluster).DeepCopy()
+					newCluster := e.ObjectNew.(*infrav1.OpenStackCluster).DeepCopy()
+
+					oldCluster.Status = infrav1.OpenStackClusterStatus{}
+					newCluster.Status = infrav1.OpenStackClusterStatus{}
+
+					oldCluster.ObjectMeta.ResourceVersion = ""
+					newCluster.ObjectMeta.ResourceVersion = ""
+
+					return !reflect.DeepEqual(oldCluster, newCluster)
+				},
+			},
+		).
+		Build(r)
+	if err != nil {
+		return errors.Wrap(err, "error creating controller")
+	}
+
+	return controller.Watch(
+		&source.Kind{Type: &clusterv1.Cluster{}},
+		&handler.EnqueueRequestsFromMapFunc{
+			ToRequests: handler.ToRequestsFunc(r.requeueOpenStackClusterForUnpausedCluster),
+		},
+		predicate.Funcs{
+			UpdateFunc: func(e event.UpdateEvent) bool {
+				oldCluster := e.ObjectOld.(*clusterv1.Cluster)
+				newCluster := e.ObjectNew.(*clusterv1.Cluster)
+				log := r.Log.WithValues("predicate", "updateEvent", "namespace", newCluster.Namespace, "cluster", newCluster.Name)
+				switch {
+				// return true if Cluster.Spec.Paused has changed from true to false
+				case oldCluster.Spec.Paused && !newCluster.Spec.Paused:
+					log.V(4).Info("Cluster was unpaused, will attempt to map associated OpenStackCluster.")
+					return true
+				// otherwise, return false
+				default:
+					log.V(4).Info("Cluster did not match expected conditions, will not attempt to map associated OpenStackCluster.")
+					return false
+				}
+			},
+			CreateFunc: func(e event.CreateEvent) bool {
+				cluster := e.Object.(*clusterv1.Cluster)
+				log := r.Log.WithValues("predicate", "createEvent", "namespace", cluster.Namespace, "cluster", cluster.Name)
+
+				// Only need to trigger a reconcile if the Cluster.Spec.Paused is false
+				if !cluster.Spec.Paused {
+					log.V(4).Info("Cluster is not paused, will attempt to map associated OpenStackCluster.")
+					return true
+				}
+				log.V(4).Info("Cluster did not match expected conditions, will not attempt to map associated OpenStackCluster.")
+				return false
+			},
+			DeleteFunc: func(e event.DeleteEvent) bool {
+				log := r.Log.WithValues("predicate", "deleteEvent", "namespace", e.Meta.GetNamespace(), "cluster", e.Meta.GetName())
+				log.V(4).Info("Cluster did not match expected conditions, will not attempt to map associated OpenStackCluster.")
+				return false
+			},
+			GenericFunc: func(e event.GenericEvent) bool {
+				log := r.Log.WithValues("predicate", "genericEvent", "namespace", e.Meta.GetNamespace(), "cluster", e.Meta.GetName())
+				log.V(4).Info("Cluster did not match expected conditions, will not attempt to map associated OpenStackCluster.")
+				return false
+			},
+		},
+	)
+}
+
+func (r *OpenStackClusterReconciler) requeueOpenStackClusterForUnpausedCluster(o handler.MapObject) []ctrl.Request {
+	c := o.Object.(*clusterv1.Cluster)
+	log := r.Log.WithValues("objectMapper", "clusterToOpenStackCluster", "namespace", c.Namespace, "cluster", c.Name)
+
+	// Don't handle deleted clusters
+	if !c.ObjectMeta.DeletionTimestamp.IsZero() {
+		log.V(4).Info("Cluster has a deletion timestamp, skipping mapping.")
+		return nil
+	}
+
+	// Make sure the ref is set
+	if c.Spec.InfrastructureRef == nil {
+		log.V(4).Info("Cluster does not have an InfrastructureRef, skipping mapping.")
+		return nil
+	}
+
+	if c.Spec.InfrastructureRef.GroupVersionKind().Kind != "OpenStackCluster" {
+		log.V(4).Info("Cluster has an InfrastructureRef for a different type, skipping mapping.")
+		return nil
+	}
+
+	log.V(4).Info("Adding request.", "openstackCluster", c.Spec.InfrastructureRef.Name)
+	return []ctrl.Request{
+		{
+			NamespacedName: client.ObjectKey{Namespace: c.Namespace, Name: c.Spec.InfrastructureRef.Name},
+		},
+	}
 }