Merge pull request kubernetes-retired#197 from erikgb/refactor/conditions

Use in-tree metav1.Condition instead of custom copied type

Author: k8s-ci-robot

api/v1alpha2/hierarchy_types.go

@@ -16,8 +16,6 @@ limitations under the License.
 package v1alpha2
 
 import (
-	"fmt"
-
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
@@ -152,7 +150,7 @@ type HierarchyConfigurationStatus struct {
 	Children []string `json:"children,omitempty"`
 
 	// Conditions describes the errors, if any.
-	Conditions []Condition `json:"conditions,omitempty"`
+	Conditions []metav1.Condition `json:"conditions,omitempty"`
 }
 
 // +kubebuilder:object:root=true
@@ -176,94 +174,6 @@ type MetaKVP struct {
 	Value string `json:"value"`
 }
 
-// metav1.Condition is introduced in k8s.io/apimachinery v0.20.0-alpha.1 and we
-// don't want to take a dependency on it yet, thus we copied the below struct from
-// https://github.com/kubernetes/apimachinery/blob/master/pkg/apis/meta/v1/types.go:
-
-// 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"`
-//
-//     // other fields
-// }
-type Condition struct {
-	// 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)
-	// +required
-	// +kubebuilder:validation:Required
-	// +kubebuilder:validation: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])$`
-	// +kubebuilder:validation:MaxLength=316
-	Type string `json:"type" protobuf:"bytes,1,opt,name=type"`
-	// status of the condition, one of True, False, Unknown.
-	// +required
-	// +kubebuilder:validation:Required
-	// +kubebuilder:validation:Enum=True;False;Unknown
-	Status metav1.ConditionStatus `json:"status" protobuf:"bytes,2,opt,name=status"`
-	// 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.
-	// +optional
-	// +kubebuilder:validation:Minimum=0
-	ObservedGeneration int64 `json:"observedGeneration,omitempty" protobuf:"varint,3,opt,name=observedGeneration"`
-	// 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.
-	// +required
-	// +kubebuilder:validation:Required
-	// +kubebuilder:validation:Type=string
-	// +kubebuilder:validation:Format=date-time
-	LastTransitionTime metav1.Time `json:"lastTransitionTime" protobuf:"bytes,4,opt,name=lastTransitionTime"`
-	// 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.
-	// +required
-	// +kubebuilder:validation:Required
-	// +kubebuilder:validation:MaxLength=1024
-	// +kubebuilder:validation:MinLength=1
-	// +kubebuilder:validation:Pattern=`^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$`
-	Reason string `json:"reason" protobuf:"bytes,5,opt,name=reason"`
-	// message is a human readable message indicating details about the transition.
-	// This may be an empty string.
-	// +required
-	// +kubebuilder:validation:Required
-	// +kubebuilder:validation:MaxLength=32768
-	Message string `json:"message" protobuf:"bytes,6,opt,name=message"`
-}
-
-// NewCondition fills some required field with default values for schema
-// validation, e.g. Status and LastTransitionTime.
-func NewCondition(tp, reason, msg string) Condition {
-	return Condition{
-		Type:   tp,
-		Status: "True",
-		// Set time as an obviously wrong value 1970-01-01T00:00:00Z since we
-		// overwrite conditions every time.
-		LastTransitionTime: metav1.Unix(0, 0),
-		Reason:             reason,
-		Message:            msg,
-	}
-}
-
-func (c Condition) String() string {
-	msg := c.Message
-	if len(msg) > 100 {
-		msg = msg[:100] + "..."
-	}
-	return fmt.Sprintf("%s (%s): %s", c.Type, c.Reason, msg)
-}
-
 func init() {
 	SchemeBuilder.Register(&HierarchyConfiguration{}, &HierarchyConfigurationList{})
 }

api/v1alpha2/hnc_config.go

@@ -159,7 +159,7 @@ type HNCConfigurationStatus struct {
 	// affected namespaces. The HierarchyConfiguration object in each of the
 	// affected namespaces will have more information. To learn more about
 	// conditions, see https://github.com/kubernetes-sigs/hierarchical-namespaces/blob/master/docs/user-guide/concepts.md#admin-conditions.
-	Conditions []Condition `json:"conditions,omitempty"`
+	Conditions []metav1.Condition `json:"conditions,omitempty"`
 }
 
 // +kubebuilder:object:root=true

api/v1alpha2/zz_generated.deepcopy.go

@@ -5,6 +5,7 @@ import (
 	"strconv"
 	"strings"
 
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime/schema"
@@ -46,7 +47,7 @@ type Namespace struct {
 
 	// conditions store conditions so that object propagation can be disabled if there's a problem
 	// on this namespace.
-	conditions []api.Condition
+	conditions []metav1.Condition
 
 	// IsSub indicates that this namespace is being or was created solely to live as a
 	// subnamespace of the specified parent.

internal/forest/namespace.go

@@ -3,6 +3,9 @@ package forest
 import (
 	"fmt"
 
+	"k8s.io/apimachinery/pkg/api/meta"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
 	api "sigs.k8s.io/hierarchical-namespaces/api/v1alpha2"
 )
 
@@ -41,7 +44,13 @@ func (ns *Namespace) SetCondition(tp, reason, msg string) {
 	if reason == api.ReasonAncestor {
 		return
 	}
-	ns.conditions = append(ns.conditions, api.NewCondition(tp, reason, msg))
+	c := metav1.Condition{
+		Type:    tp,
+		Status:  metav1.ConditionTrue,
+		Reason:  reason,
+		Message: msg,
+	}
+	meta.SetStatusCondition(&ns.conditions, c)
 }
 
 // ClearConditions set conditions to nil.
@@ -50,12 +59,19 @@ func (ns *Namespace) ClearConditions() {
 }
 
 // Conditions returns a full list of the conditions in the namespace.
-func (ns *Namespace) Conditions() []api.Condition {
+func (ns *Namespace) Conditions() []metav1.Condition {
 	if root := ns.GetHaltedRoot(); root != "" && root != ns.name {
-		msg := fmt.Sprintf("Propagation paused in %q and its descendants due to ActivitiesHalted condition on ancestor %q", ns.name, root)
-		ret := []api.Condition{api.NewCondition(api.ConditionActivitiesHalted, api.ReasonAncestor, msg)}
+		ret := []metav1.Condition{{
+			Type:   api.ConditionActivitiesHalted,
+			Status: metav1.ConditionTrue,
+			// TODO(adrianludwin) Review; Some callers require this field to be set - since it is mandatory in the API
+			// Set time as an obviously wrong value 1970-01-01T00:00:00Z since we
+			// overwrite conditions every time.
+			LastTransitionTime: metav1.Unix(0, 0),
+			Reason:             api.ReasonAncestor,
+			Message:            fmt.Sprintf("Propagation paused in %q and its descendants due to ActivitiesHalted condition on ancestor %q", ns.name, root),
+		}}
 		return append(ret, ns.conditions...)
 	}
-
 	return ns.conditions
 }

internal/forest/namespaceconditions.go

@@ -11,6 +11,7 @@ import (
 	apiextensions "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -357,7 +358,13 @@ func (r *Reconciler) createObjectReconciler(gvk schema.GroupVersionKind, mode ap
 }
 
 func (r *Reconciler) writeCondition(inst *api.HNCConfiguration, tp, reason, msg string) {
-	inst.Status.Conditions = append(inst.Status.Conditions, api.NewCondition(tp, reason, msg))
+	c := metav1.Condition{
+		Type:    tp,
+		Status:  metav1.ConditionTrue,
+		Reason:  reason,
+		Message: msg,
+	}
+	meta.SetStatusCondition(&inst.Status.Conditions, c)
 }
 
 // setTypeStatuses adds Status.Resources for types configured in the spec. Only the status of types

internal/hncconfig/reconciler.go

@@ -28,7 +28,6 @@ import (
 	"k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/duration"
-	api "sigs.k8s.io/hierarchical-namespaces/api/v1alpha2"
 )
 
 var describeCmd = &cobra.Command{
@@ -83,7 +82,7 @@ var describeCmd = &cobra.Command{
 	},
 }
 
-func describeConditions(cond []api.Condition) {
+func describeConditions(cond []metav1.Condition) {
 	if len(cond) == 0 {
 		fmt.Printf("  No conditions\n")
 		return

internal/kubectl/describe.go

@@ -24,7 +24,7 @@ import (
 
 	"github.com/go-logr/logr"
 	"k8s.io/apimachinery/pkg/api/errors"
-	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
@@ -310,7 +310,7 @@ func (r *Reconciler) skipNamespace(log logr.Logger, inst *unstructured.Unstructu
 // modify `inst`.
 func (r *Reconciler) syncMissingObject(log logr.Logger, inst *unstructured.Unstructured) syncAction {
 	// If the object exists, skip.
-	if inst.GetCreationTimestamp() != (v1.Time{}) {
+	if inst.GetCreationTimestamp() != (metav1.Time{}) {
 		return actionUnknown
 	}
 
@@ -435,7 +435,7 @@ func (r *Reconciler) syncPropagated(inst, srcInst *unstructured.Unstructured) (s
 	}
 
 	// If an object doesn't exist, assume it's been deleted or not yet created.
-	exists := inst.GetCreationTimestamp() != v1.Time{}
+	exists := inst.GetCreationTimestamp() != metav1.Time{}
 
 	// If the copy does not exist, or is different from the source, return the write action and the
 	// source instance. Note that DeepEqual could return `true` even if the object doesn't exist if
@@ -594,7 +594,7 @@ func (r *Reconciler) deleteObject(ctx context.Context, log logr.Logger, inst *un
 
 func (r *Reconciler) writeObject(ctx context.Context, log logr.Logger, inst, srcInst *unstructured.Unstructured) error {
 	// The object exists if CreationTimestamp is set. This flag enables us to have only 1 API call.
-	exist := inst.GetCreationTimestamp() != v1.Time{}
+	exist := inst.GetCreationTimestamp() != metav1.Time{}
 	ns := inst.GetNamespace()
 	// Get current ResourceVersion, required for updates for newer API objects (including custom resources).
 	rv := inst.GetResourceVersion()

internal/objects/reconciler.go

@@ -7,7 +7,7 @@ import (
 	"strconv"
 	"strings"
 
-	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/util/validation"
@@ -126,11 +126,11 @@ func GetSelector(inst *unstructured.Unstructured) (labels.Selector, error) {
 // getSelectorFromString converts the given string to a selector
 // Note: any invalid Selector value will cause this object not propagating to any child namespace
 func getSelectorFromString(str string) (labels.Selector, error) {
-	labelSelector, err := v1.ParseToLabelSelector(str)
+	labelSelector, err := metav1.ParseToLabelSelector(str)
 	if err != nil {
 		return nil, err
 	}
-	selector, err := v1.LabelSelectorAsSelector(labelSelector)
+	selector, err := metav1.LabelSelectorAsSelector(labelSelector)
 	if err != nil {
 		return nil, err
 	}