From 41af89f0e4fbe738e458d5d51a329e8f96a688f6 Mon Sep 17 00:00:00 2001 From: Sean Eagan Date: Thu, 7 Jan 2021 16:03:23 -0600 Subject: [PATCH] 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 | 2 +- api/v1/conditions.go | 27 ++++++++ api/v1/vino_types.go | 15 +---- api/v1/zz_generated.deepcopy.go | 22 ++----- .../bases/airship.airshipit.org_vinoes.yaml | 62 +++++++++++++++++-- controllers/vino_controller.go | 52 +++++++--------- go.mod | 6 +- go.sum | 6 ++ main.go | 8 +-- 9 files changed, 130 insertions(+), 70 deletions(-) create mode 100644 api/v1/conditions.go diff --git a/Makefile b/Makefile index 391008c..03a9abd 100644 --- a/Makefile +++ b/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 diff --git a/api/v1/conditions.go b/api/v1/conditions.go new file mode 100644 index 0000000..ce71eba --- /dev/null +++ b/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" +) diff --git a/api/v1/vino_types.go b/api/v1/vino_types.go index 1783e04..dcbe654 100644 --- a/api/v1/vino_types.go +++ b/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" ) diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go index 4106901..d58e562 100644 --- a/api/v1/zz_generated.deepcopy.go +++ b/api/v1/zz_generated.deepcopy.go @@ -21,6 +21,7 @@ limitations under the License. package v1 import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" ) @@ -39,21 +40,6 @@ func (in *CPUConfiguration) DeepCopy() *CPUConfiguration { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *Condition) DeepCopyInto(out *Condition) { - *out = *in -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Condition. -func (in *Condition) DeepCopy() *Condition { - if in == nil { - return nil - } - out := new(Condition) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *DaemonSetOptions) DeepCopyInto(out *DaemonSetOptions) { *out = *in @@ -360,8 +346,10 @@ func (in *VinoStatus) DeepCopyInto(out *VinoStatus) { out.ConfigMapRef = in.ConfigMapRef if in.Conditions != nil { in, out := &in.Conditions, &out.Conditions - *out = make([]Condition, len(*in)) - copy(*out, *in) + *out = make([]metav1.Condition, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } } } diff --git a/config/crd/bases/airship.airshipit.org_vinoes.yaml b/config/crd/bases/airship.airshipit.org_vinoes.yaml index d543145..3e2d6ad 100644 --- a/config/crd/bases/airship.airshipit.org_vinoes.yaml +++ b/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: - message: + 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: diff --git a/controllers/vino_controller.go b/controllers/vino_controller.go index 5dd17be..9c4aa9d 100644 --- a/controllers/vino_controller.go +++ b/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,13 +98,15 @@ 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 } @@ -111,13 +114,15 @@ func (r *VinoReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl. 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 diff --git a/go.mod b/go.mod index 1d72b07..5c22847 100644 --- a/go.mod +++ b/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 ) diff --git a/go.sum b/go.sum index 7ce5869..2d7977b 100644 --- a/go.sum +++ b/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= diff --git a/main.go b/main.go index a6f9212..5d10503 100644 --- a/main.go +++ b/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")