From bea81ae7b291615261944b9c36a955911e78b659 Mon Sep 17 00:00:00 2001 From: Aurel Canciu Date: Sun, 1 Nov 2020 19:51:44 +0200 Subject: [PATCH 1/3] Refactor to use new metav1.Condition Use the newly introduced standardized Condition type kubernetes/enhancements#1624 Signed-off-by: Aurel Canciu --- apis/meta/condition_types.go | 127 ----------------------------------- apis/meta/conditions.go | 69 +++++++++++++++++++ apis/meta/conditions_test.go | 95 ++++++++++++++++++++++++++ apis/meta/go.mod | 5 +- apis/meta/go.sum | 2 - 5 files changed, 165 insertions(+), 133 deletions(-) delete mode 100644 apis/meta/condition_types.go create mode 100644 apis/meta/conditions.go create mode 100644 apis/meta/conditions_test.go diff --git a/apis/meta/condition_types.go b/apis/meta/condition_types.go deleted file mode 100644 index 2391f8d2..00000000 --- a/apis/meta/condition_types.go +++ /dev/null @@ -1,127 +0,0 @@ -/* -Copyright 2020 The Flux authors - -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 meta - -import ( - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" -) - -// Condition contains condition information of a toolkit resource. -type Condition struct { - // Type of the condition. - // +required - Type string `json:"type"` - - // Status of the condition, one of ('True', 'False', 'Unknown'). - // +required - Status corev1.ConditionStatus `json:"status"` - - // LastTransitionTime is the timestamp corresponding to the last status - // change of this condition. - // +required - LastTransitionTime metav1.Time `json:"lastTransitionTime,omitempty"` - - // Reason is a brief machine readable explanation for the condition's last - // transition. - // +required - Reason string `json:"reason,omitempty"` - - // Message is a human readable description of the details of the last - // transition, complementing reason. - // +optional - Message string `json:"message,omitempty"` -} - -const ( - // ReadyCondition is the name of the Ready condition implemented by all toolkit - // resources. - ReadyCondition string = "Ready" -) - -const ( - // ReconciliationSucceededReason represents the fact that the reconciliation of - // a toolkit resource has succeeded. - ReconciliationSucceededReason string = "ReconciliationSucceeded" - - // ReconciliationFailedReason represents the fact that the reconciliation of a - // toolkit resource has failed. - ReconciliationFailedReason string = "ReconciliationFailed" - - // ProgressingReason represents the fact that the reconciliation of a toolkit - // resource is underway. - ProgressingReason string = "Progressing" - - // DependencyNotReadyReason represents the fact that one of the toolkit resource - // dependencies is not ready. - DependencyNotReadyReason string = "DependencyNotReady" - - // SuspendedReason represents the fact that the reconciliation of a toolkit - // resource is suspended. - SuspendedReason string = "Suspended" -) - -// DeepCopyInto is a deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *Condition) DeepCopyInto(out *Condition) { - *out = *in - in.LastTransitionTime.DeepCopyInto(&out.LastTransitionTime) -} - -// DeepCopy is a 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 -} - -// GetCondition returns the Condition from the given slice that matches the -// given type. -func GetCondition(conditions []Condition, conditionType string) *Condition { - for i := range conditions { - c := conditions[i] - if c.Type == conditionType { - return &c - } - } - return nil -} - -// HasReadyCondition returns if the given Condition slice has a ReadyCondition -// with a 'True' condition status. -func HasReadyCondition(conditions []Condition) bool { - condition := GetCondition(conditions, ReadyCondition) - if condition == nil { - return false - } - return condition.Status == corev1.ConditionTrue -} - -// FilterOutCondition returns a new Condition slice without the Condition of the -// given type. -func FilterOutCondition(conditions []Condition, conditionType string) []Condition { - var newConditions []Condition - for _, c := range conditions { - if c.Type == conditionType { - continue - } - newConditions = append(newConditions, c) - } - return newConditions -} diff --git a/apis/meta/conditions.go b/apis/meta/conditions.go new file mode 100644 index 00000000..8f691e4a --- /dev/null +++ b/apis/meta/conditions.go @@ -0,0 +1,69 @@ +/* +Copyright 2020 The Flux authors + +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 meta + +import ( + apimeta "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +const ( + // ReadyCondition is the name of the Ready condition implemented by all toolkit + // resources. + ReadyCondition string = "Ready" +) + +const ( + // ReconciliationSucceededReason represents the fact that the reconciliation of + // a toolkit resource has succeeded. + ReconciliationSucceededReason string = "ReconciliationSucceeded" + + // ReconciliationFailedReason represents the fact that the reconciliation of a + // toolkit resource has failed. + ReconciliationFailedReason string = "ReconciliationFailed" + + // ProgressingReason represents the fact that the reconciliation of a toolkit + // resource is underway. + ProgressingReason string = "Progressing" + + // DependencyNotReadyReason represents the fact that one of the toolkit resource + // dependencies is not ready. + DependencyNotReadyReason string = "DependencyNotReady" + + // SuspendedReason represents the fact that the reconciliation of a toolkit + // resource is suspended. + SuspendedReason string = "Suspended" +) + +// InReadyCondition returns if the given Condition slice has a ReadyCondition +// with a 'True' condition status. +func InReadyCondition(conditions []metav1.Condition) bool { + return apimeta.IsStatusConditionTrue(conditions, ReadyCondition) +} + +// FilterOutCondition returns a new Condition slice without the Condition of the +// given type. +func FilterOutCondition(conditions []metav1.Condition, conditionType string) []metav1.Condition { + var newConditions []metav1.Condition + for _, c := range conditions { + if c.Type == conditionType { + continue + } + newConditions = append(newConditions, c) + } + return newConditions +} diff --git a/apis/meta/conditions_test.go b/apis/meta/conditions_test.go new file mode 100644 index 00000000..d2b865c0 --- /dev/null +++ b/apis/meta/conditions_test.go @@ -0,0 +1,95 @@ +/* +Copyright 2020 The Flux authors + +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 meta + +import ( + "testing" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestInReadyCondition(t *testing.T) { + var conditions []metav1.Condition + + found := InReadyCondition(conditions) + if found { + t.Error("expected InReadyCondition to return false when no conditions") + } + + fake := metav1.Condition{ + Type: "FakeCondition", + } + conditions = append(conditions, fake) + found = InReadyCondition(conditions) + if found { + t.Error("expected InReadyCondition to return false when no ReadyCondition exists") + } + + ready := metav1.Condition{ + Type: ReadyCondition, + Status: metav1.ConditionFalse, + } + conditions = append(conditions, ready) + found = InReadyCondition(conditions) + if found { + t.Error("expected InReadyCondition to return false if ReadyCondition Status is false") + } + + ready.Status = metav1.ConditionTrue + conditions = []metav1.Condition{ready} + found = InReadyCondition(conditions) + if !found { + t.Error("expected InReadyCondition to return true") + } +} + +func TestFilterOutCondition(t *testing.T) { + const FakeCondition = "FakeCondition" + var conditions []metav1.Condition + + filtered := FilterOutCondition(conditions, "") + if len(filtered) > 0 { + t.Error("expected FilterOutCondition to return empty slice") + } + + fake := metav1.Condition{ + Type: FakeCondition, + } + conditions = append(conditions, fake) + filtered = FilterOutCondition(conditions, FakeCondition) + if len(filtered) > 0 { + t.Error("expected FilterOutCondition to return empty slice") + } + + ready := metav1.Condition{ + Type: ReadyCondition, + } + conditions = append(conditions, ready) + filtered = FilterOutCondition(conditions, FakeCondition) + if filtered[0] != conditions[1] { + t.Error("expected FilterOutCondition to return the ready condition") + } + + test := metav1.Condition{ + Type: "TestCondition", + } + conditions = append(conditions, test) + filtered = FilterOutCondition(conditions, FakeCondition) + if len(filtered) != 2 { + t.Error("expected FilterOutCondition to have two elements") + } +} diff --git a/apis/meta/go.mod b/apis/meta/go.mod index 26cba469..ef7eefff 100644 --- a/apis/meta/go.mod +++ b/apis/meta/go.mod @@ -2,7 +2,4 @@ module github.com/fluxcd/pkg/apis/meta go 1.15 -require ( - k8s.io/api v0.19.3 - k8s.io/apimachinery v0.19.3 -) +require k8s.io/apimachinery v0.19.3 diff --git a/apis/meta/go.sum b/apis/meta/go.sum index bf720530..aaf2df80 100644 --- a/apis/meta/go.sum +++ b/apis/meta/go.sum @@ -156,8 +156,6 @@ gopkg.in/yaml.v2 v2.2.8 h1:obN1ZagJSUGI0Ek/LBmuj4SNLPfIny3KsKFopxRdj10= gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= -k8s.io/api v0.19.3 h1:GN6ntFnv44Vptj/b+OnMW7FmzkpDoIDLZRvKX3XH9aU= -k8s.io/api v0.19.3/go.mod h1:VF+5FT1B74Pw3KxMdKyinLo+zynBaMBiAfGMuldcNDs= k8s.io/apimachinery v0.19.3 h1:bpIQXlKjB4cB/oNpnNnV+BybGPR7iP5oYpsOTEJ4hgc= k8s.io/apimachinery v0.19.3/go.mod h1:DnPGDnARWFvYa3pMHgSxtbZb7gpzzAZ1pTfaUNDVlmA= k8s.io/gengo v0.0.0-20200413195148-3a45101e95ac/go.mod h1:ezvh/TsK7cY6rbqRK0oQQ8IAqLxYwwyPxAX1Pzy0ii0= From d8fe7f2b4485b5e992007e94b193fd8036dd22ba Mon Sep 17 00:00:00 2001 From: Aurel Canciu Date: Sun, 1 Nov 2020 19:54:35 +0200 Subject: [PATCH 2/3] Don't publicize annotations_test mock types Signed-off-by: Aurel Canciu --- apis/meta/annotations_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/apis/meta/annotations_test.go b/apis/meta/annotations_test.go index 3978a0e5..f923613d 100644 --- a/apis/meta/annotations_test.go +++ b/apis/meta/annotations_test.go @@ -21,17 +21,17 @@ import ( "time" ) -type WhateverStatus struct { +type whateverStatus struct { ReconcileRequestStatus `json:",inline"` } -type Whatever struct { +type whatever struct { Annotations map[string]string - Status WhateverStatus `json:"status,omitempty"` + Status whateverStatus `json:"status,omitempty"` } func TestGetAnnotationValue(t *testing.T) { - obj := Whatever{ + obj := whatever{ Annotations: map[string]string{}, } From 20138fb4337db24752e14d865e03d08032194869 Mon Sep 17 00:00:00 2001 From: Aurel Canciu Date: Sun, 1 Nov 2020 20:02:42 +0200 Subject: [PATCH 3/3] Refactor runtime to use new standard conditions Signed-off-by: Aurel Canciu --- runtime/metrics/recorder.go | 6 +++--- runtime/metrics/recorder_test.go | 7 ++++--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/runtime/metrics/recorder.go b/runtime/metrics/recorder.go index 8b21ad67..e03df6bc 100644 --- a/runtime/metrics/recorder.go +++ b/runtime/metrics/recorder.go @@ -6,7 +6,7 @@ import ( "github.com/prometheus/client_golang/prometheus" corev1 "k8s.io/api/core/v1" - "github.com/fluxcd/pkg/apis/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) const ( @@ -42,8 +42,8 @@ func (r *Recorder) Collectors() []prometheus.Collector { return []prometheus.Collector{r.conditionGauge, r.durationHistogram} } -func (r *Recorder) RecordCondition(ref corev1.ObjectReference, condition meta.Condition, deleted bool) { - for _, status := range []string{string(corev1.ConditionTrue), string(corev1.ConditionFalse), string(corev1.ConditionUnknown), ConditionDeleted} { +func (r *Recorder) RecordCondition(ref corev1.ObjectReference, condition metav1.Condition, deleted bool) { + for _, status := range []string{string(metav1.ConditionTrue), string(metav1.ConditionFalse), string(corev1.ConditionUnknown), ConditionDeleted} { var value float64 if deleted { if status == ConditionDeleted { diff --git a/runtime/metrics/recorder_test.go b/runtime/metrics/recorder_test.go index e9aad972..2a9d8ef4 100644 --- a/runtime/metrics/recorder_test.go +++ b/runtime/metrics/recorder_test.go @@ -9,6 +9,7 @@ import ( corev1 "k8s.io/api/core/v1" "github.com/fluxcd/pkg/apis/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) func TestRecorder_RecordCondition(t *testing.T) { @@ -22,9 +23,9 @@ func TestRecorder_RecordCondition(t *testing.T) { Name: "test", } - cond := meta.Condition{ + cond := metav1.Condition{ Type: meta.ReadyCondition, - Status: corev1.ConditionTrue, + Status: metav1.ConditionTrue, } rec.RecordCondition(ref, cond, false) @@ -43,7 +44,7 @@ func TestRecorder_RecordCondition(t *testing.T) { if *pair.Name == "type" && *pair.Value != meta.ReadyCondition { t.Errorf("expected condition type to be %s, got %s", meta.ReadyCondition, *pair.Value) } - if *pair.Name == "status" && *pair.Value == string(corev1.ConditionTrue) { + if *pair.Name == "status" && *pair.Value == string(metav1.ConditionTrue) { conditionTrueValue = *m.GetGauge().Value } else if *pair.Name == "status" && *m.GetGauge().Value != 0 { t.Errorf("expected guage value to be 0, got %v", *m.GetGauge().Value)