Use kubernetes standardized status conditions

This integrates the standardized status condition schema instituted here:
kubernetes/enhancements#1624

Which is available in the kubernetes 1.19 libraries:
https://github.com/kubernetes/apimachinery/blob/release-1.19/pkg/apis/meta/v1/types.go

As well as the helper libraries added here:
kubernetes/kubernetes#92717

Note there is a bug with observed generation handling, which is fixed in the kubernetes 0.20 libraries:
https: //github.com/kubernetes/kubernetes/commit/5712e33abcea86e7bf699f40a3a838afc1b7c278

This also updates controller-gen from v0.2.5 > v0.3.0

Change-Id: Ib84f0fa2b261fe6ae2568fc8227cb67df1a21260

Makefile

@@ -82,7 +82,7 @@ ifeq (, $(shell which controller-gen))
 	CONTROLLER_GEN_TMP_DIR=$$(mktemp -d) ;\
 	cd $$CONTROLLER_GEN_TMP_DIR ;\
 	go mod init tmp ;\
-	go get sigs.k8s.io/controller-tools/cmd/controller-gen@v0.2.5 ;\
+	go get sigs.k8s.io/controller-tools/cmd/controller-gen@v0.3.0 ;\
 	rm -rf $$CONTROLLER_GEN_TMP_DIR ;\
 	}
 CONTROLLER_GEN=$(GOBIN)/controller-gen

api/v1/conditions.go

@@ -0,0 +1,27 @@
+/*
+
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package v1
+
+const (
+	// ReconciliationSucceededReason represents the fact that the reconciliation of
+	// the resource has succeeded.
+	ReconciliationSucceededReason string = "ReconciliationSucceeded"
+
+	// ReconciliationFailedReason represents the fact that the reconciliation of
+	// the resource has failed.
+	ReconciliationFailedReason string = "ReconciliationFailed"
+)

api/v1/vino_types.go

@@ -152,24 +152,13 @@ func init() {
 // VinoStatus defines the observed state of Vino
 type VinoStatus struct {
 	ConfigMapRef         corev1.ObjectReference `json:"configMapRef,omitempty"`
-	Conditions           []Condition            `json:"conditions,omitempty"`
+	Conditions           []metav1.Condition     `json:"conditions,omitempty"`
 	ConfigMapReady       bool                   `json:"configMapReady,omitempty"`
 	VirtualMachinesReady bool                   `json:"virtualMachinesReady,omitempty"`
 	NetworkingReady      bool                   `json:"networkingReady,omitempty"`
 	DaemonSetReady       bool                   `json:"daemonSetReady,omitempty"`
 }
 
-// Condition indicates operational status of VINO CR
-type Condition struct {
-	Status  corev1.ConditionStatus `json:"status,omitempty"`
-	Type    ConditionType          `json:"type,omitempty"`
-	Reason  string                 `json:"reason,omitempty"`
-	Message string                 `json:"message,omitempty"`
-}
-
-// ConditionType type of the condition
-type ConditionType string
-
 const (
-	ConditionTypeReady ConditionType = "Ready"
+	ConditionTypeReady string = "Ready"
 )

config/crd/bases/airship.airshipit.org_vinoes.yaml

@@ -4,7 +4,7 @@ apiVersion: apiextensions.k8s.io/v1beta1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.2.5
+    controller-gen.kubebuilder.io/version: v0.3.0
   creationTimestamp: null
   name: vinoes.airship.airshipit.org
 spec:
@@ -169,17 +169,71 @@ spec:
           properties:
             conditions:
               items:
-                description: Condition indicates operational status of VINO CR
+                description: "Condition contains details for one aspect of the current
+                  state of this API Resource. --- This struct is intended for direct
+                  use as an array at the field path .status.conditions.  For example,
+                  type FooStatus struct{     // Represents the observations of a foo's
+                  current state.     // Known .status.conditions.type are: \"Available\",
+                  \"Progressing\", and \"Degraded\"     // +patchMergeKey=type     //
+                  +patchStrategy=merge     // +listType=map     // +listMapKey=type
+                  \    Conditions []metav1.Condition `json:\"conditions,omitempty\"
+                  patchStrategy:\"merge\" patchMergeKey:\"type\" protobuf:\"bytes,1,rep,name=conditions\"`
+                  \n     // other fields }"
                 properties:
+                  lastTransitionTime:
+                    description: lastTransitionTime is the last time the condition
+                      transitioned from one status to another. This should be when
+                      the underlying condition changed.  If that is not known, then
+                      using the time when the API field changed is acceptable.
+                    format: date-time
+                    type: string
                   message:
+                    description: message is a human readable message indicating details
+                      about the transition. This may be an empty string.
+                    maxLength: 32768
                     type: string
+                  observedGeneration:
+                    description: observedGeneration represents the .metadata.generation
+                      that the condition was set based upon. For instance, if .metadata.generation
+                      is currently 12, but the .status.conditions[x].observedGeneration
+                      is 9, the condition is out of date with respect to the current
+                      state of the instance.
+                    format: int64
+                    minimum: 0
+                    type: integer
                   reason:
+                    description: reason contains a programmatic identifier indicating
+                      the reason for the condition's last transition. Producers of
+                      specific condition types may define expected values and meanings
+                      for this field, and whether the values are considered a guaranteed
+                      API. The value should be a CamelCase string. This field may
+                      not be empty.
+                    maxLength: 1024
+                    minLength: 1
+                    pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$
                     type: string
                   status:
+                    description: status of the condition, one of True, False, Unknown.
+                    enum:
+                    - "True"
+                    - "False"
+                    - Unknown
                     type: string
                   type:
-                    description: ConditionType type of the condition
+                    description: type of condition in CamelCase or in foo.example.com/CamelCase.
+                      --- Many .condition.type values are consistent across resources
+                      like Available, but because arbitrary conditions can be useful
+                      (see .node.status.conditions), the ability to deconflict is
+                      important. The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt)
+                    maxLength: 316
+                    pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$
                     type: string
+                required:
+                - lastTransitionTime
+                - message
+                - reason
+                - status
+                - type
                 type: object
               type: array
             configMapReady:

controllers/vino_controller.go

@@ -25,6 +25,7 @@ import (
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	apierror "k8s.io/apimachinery/pkg/api/errors"
+	apimeta "k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
@@ -97,27 +98,31 @@ func (r *VinoReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.
 
 	err = r.ensureConfigMap(ctx, req.NamespacedName, vino)
 	if err != nil {
-		readyCondition := vinov1.Condition{
-			Status:  corev1.ConditionFalse,
-			Reason:  "Error has occurred while making sure that ConfigMap for VINO is in correct state",
-			Message: err.Error(),
-			Type:    vinov1.ConditionTypeReady,
+		err = fmt.Errorf("Could not reconcile ConfigMap: %w", err)
+		readyCondition := metav1.Condition{
+			Status:             metav1.ConditionFalse,
+			Reason:             vinov1.ReconciliationFailedReason,
+			Message:            err.Error(),
+			Type:               vinov1.ConditionTypeReady,
+			ObservedGeneration: vino.GetGeneration(),
 		}
-		r.setCondition(vino, readyCondition)
+		apimeta.SetStatusCondition(&vino.Status.Conditions, readyCondition)
 		vino.Status.ConfigMapReady = false
 		return ctrl.Result{Requeue: true, RequeueAfter: time.Second * 60}, err
 	}
 	vino.Status.ConfigMapReady = true
 
 	err = r.ensureDaemonSet(ctx, req.NamespacedName, vino)
 	if err != nil {
-		readyCondition := vinov1.Condition{
-			Status:  corev1.ConditionFalse,
-			Reason:  "Error has occurred while making sure that VINO Daemonset is installed on kubernetes nodes",
-			Message: err.Error(),
-			Type:    vinov1.ConditionTypeReady,
+		err = fmt.Errorf("Could not reconcile Daemonset: %w", err)
+		readyCondition := metav1.Condition{
+			Status:             metav1.ConditionFalse,
+			Reason:             vinov1.ReconciliationFailedReason,
+			Message:            err.Error(),
+			Type:               vinov1.ConditionTypeReady,
+			ObservedGeneration: vino.GetGeneration(),
 		}
-		r.setCondition(vino, readyCondition)
+		apimeta.SetStatusCondition(&vino.Status.Conditions, readyCondition)
 		vino.Status.DaemonSetReady = false
 		return ctrl.Result{Requeue: true, RequeueAfter: time.Second * 60}, err
 	}
@@ -200,13 +205,14 @@ func (r *VinoReconciler) getCurrentConfigMap(ctx context.Context, vino *vinov1.V
 func (r *VinoReconciler) setReadyStatus(vino *vinov1.Vino) {
 	if vino.Status.ConfigMapReady && vino.Status.DaemonSetReady {
 		r.Log.Info("All VINO components are in ready state, setting VINO CR to ready state")
-		readyCondition := vinov1.Condition{
-			Status:  corev1.ConditionTrue,
-			Reason:  "Networking, Virtual Machines, DaemonSet and ConfigMap is in ready state",
-			Message: "All VINO components are in ready state, setting VINO CR to ready state",
-			Type:    vinov1.ConditionTypeReady,
+		readyCondition := metav1.Condition{
+			Status:             metav1.ConditionTrue,
+			Reason:             vinov1.ReconciliationSucceededReason,
+			Message:            "All VINO components are in ready state, setting VINO CR to ready state",
+			Type:               vinov1.ConditionTypeReady,
+			ObservedGeneration: vino.GetGeneration(),
 		}
-		r.setCondition(vino, readyCondition)
+		apimeta.SetStatusCondition(&vino.Status.Conditions, readyCondition)
 	}
 }
 
@@ -381,16 +387,6 @@ func (r *VinoReconciler) finalize(ctx context.Context, vino *vinov1.Vino) error
 	return r.Update(ctx, vino)
 }
 
-func (r *VinoReconciler) setCondition(vino *vinov1.Vino, condition vinov1.Condition) {
-	for i, checkCondition := range vino.Status.Conditions {
-		if checkCondition.Type == condition.Type {
-			vino.Status.Conditions[i] = condition
-			return
-		}
-	}
-	vino.Status.Conditions = append([]vinov1.Condition{condition}, vino.Status.Conditions...)
-}
-
 func defaultDaemonSet(vino *vinov1.Vino) (ds *appsv1.DaemonSet) {
 	libvirtImage := DefaultImageLibvirt
 

go.mod

@@ -12,9 +12,9 @@ require (
 	golang.org/x/sys v0.0.0-20200814200057-3d37ad5750ed // indirect
 	golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 // indirect
 	gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f // indirect
-	k8s.io/api v0.19.2
-	k8s.io/apimachinery v0.19.2
-	k8s.io/client-go v0.19.2
+	k8s.io/api v0.19.3
+	k8s.io/apimachinery v0.19.3
+	k8s.io/client-go v0.19.3
 	sigs.k8s.io/controller-runtime v0.7.0
 	sigs.k8s.io/yaml v1.2.0
 )

go.sum

@@ -659,13 +659,19 @@ honnef.co/go/tools v0.0.1-2019.2.3 h1:3JgtbtFHMiCmsznwGVTUWbgGov+pVqnlf1dEJTNAXe
 honnef.co/go/tools v0.0.1-2019.2.3/go.mod h1:a3bituU0lyd329TUQxRnasdCoJDkEUEAqEt0JzvZhAg=
 k8s.io/api v0.19.2 h1:q+/krnHWKsL7OBZg/rxnycsl9569Pud76UJ77MvKXms=
 k8s.io/api v0.19.2/go.mod h1:IQpK0zFQ1xc5iNIQPqzgoOwuFugaYHK4iCknlAQP9nI=
+k8s.io/api v0.19.3 h1:GN6ntFnv44Vptj/b+OnMW7FmzkpDoIDLZRvKX3XH9aU=
+k8s.io/api v0.19.3/go.mod h1:VF+5FT1B74Pw3KxMdKyinLo+zynBaMBiAfGMuldcNDs=
 k8s.io/apiextensions-apiserver v0.19.2 h1:oG84UwiDsVDu7dlsGQs5GySmQHCzMhknfhFExJMz9tA=
 k8s.io/apiextensions-apiserver v0.19.2/go.mod h1:EYNjpqIAvNZe+svXVx9j4uBaVhTB4C94HkY3w058qcg=
 k8s.io/apimachinery v0.19.2 h1:5Gy9vQpAGTKHPVOh5c4plE274X8D/6cuEiTO2zve7tc=
 k8s.io/apimachinery v0.19.2/go.mod h1:DnPGDnARWFvYa3pMHgSxtbZb7gpzzAZ1pTfaUNDVlmA=
+k8s.io/apimachinery v0.19.3 h1:bpIQXlKjB4cB/oNpnNnV+BybGPR7iP5oYpsOTEJ4hgc=
+k8s.io/apimachinery v0.19.3/go.mod h1:DnPGDnARWFvYa3pMHgSxtbZb7gpzzAZ1pTfaUNDVlmA=
 k8s.io/apiserver v0.19.2/go.mod h1:FreAq0bJ2vtZFj9Ago/X0oNGC51GfubKK/ViOKfVAOA=
 k8s.io/client-go v0.19.2 h1:gMJuU3xJZs86L1oQ99R4EViAADUPMHHtS9jFshasHSc=
 k8s.io/client-go v0.19.2/go.mod h1:S5wPhCqyDNAlzM9CnEdgTGV4OqhsW3jGO1UM1epwfJA=
+k8s.io/client-go v0.19.3 h1:ctqR1nQ52NUs6LpI0w+a5U+xjYwflFwA13OJKcicMxg=
+k8s.io/client-go v0.19.3/go.mod h1:+eEMktZM+MG0KO+PTkci8xnbCZHvj9TqR6Q1XDUIJOM=
 k8s.io/code-generator v0.19.2/go.mod h1:moqLn7w0t9cMs4+5CQyxnfA/HV8MF6aAVENF+WZZhgk=
 k8s.io/component-base v0.19.2 h1:jW5Y9RcZTb79liEhW3XDVTW7MuvEGP0tQZnfSX6/+gs=
 k8s.io/component-base v0.19.2/go.mod h1:g5LrsiTiabMLZ40AR6Hl45f088DevyGY+cCE2agEIVo=

main.go

@@ -20,14 +20,14 @@ import (
 	"flag"
 	"os"
 
+	appsv1 "k8s.io/api/apps/v1"
+	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
 	_ "k8s.io/client-go/plugin/pkg/client/auth/gcp"
 	ctrl "sigs.k8s.io/controller-runtime"
-	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 	"sigs.k8s.io/controller-runtime/pkg/client"
-	corev1 "k8s.io/api/core/v1"
-	appsv1 "k8s.io/api/apps/v1"
+	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 
 	vinov1 "vino/api/v1"
 	"vino/controllers"
@@ -67,7 +67,7 @@ func main() {
 			&corev1.ConfigMap{},
 			&appsv1.DaemonSet{},
 		},
-		LeaderElectionID:   "c3bc287f.airshipit.org",
+		LeaderElectionID: "c3bc287f.airshipit.org",
 	})
 	if err != nil {
 		setupLog.Error(err, "unable to start manager")