From a252e56bd15ed297c029de50742fe8aeead880eb Mon Sep 17 00:00:00 2001 From: Axel Christ Date: Tue, 10 May 2022 09:02:12 +0200 Subject: [PATCH] Simplify & Fix `Prefix` / `PrefixAllocation` Introducing a `Phase` for `Prefix` / `PrefixAllocation` as we have it for e.g. `VirtualIP`. This makes more sense for several reasons: * Code reading and writing the phase is much simpler * Transitioning between multiple different states and encoding it into a `Reason` field of a condition fuzzes the line of when to actually update the `LastTransitionTime` of a condition (that actually should only change when updating between `Unknown`, `True` and `False`). Fix some additional issues. --- apis/ipam/prefix_types.go | 23 +- apis/ipam/prefixallocation_types.go | 31 +-- apis/ipam/readiness.go | 100 --------- apis/ipam/v1alpha1/prefix_types.go | 36 +-- apis/ipam/v1alpha1/prefixallocation_types.go | 40 ++-- apis/ipam/v1alpha1/readiness.go | 101 --------- apis/ipam/v1alpha1/zz_generated.conversion.go | 112 +++------ apis/ipam/v1alpha1/zz_generated.deepcopy.go | 52 +---- apis/ipam/validation/prefix.go | 26 +-- apis/ipam/validation/prefix_test.go | 138 +++--------- apis/ipam/validation/prefixallocation.go | 23 +- apis/ipam/validation/prefixallocation_test.go | 79 ++----- apis/ipam/zz_generated.deepcopy.go | 52 +---- controllers/ipam/ipam.go | 18 -- controllers/ipam/prefix_controller.go | 157 ++++++------- controllers/ipam/prefix_controller_test.go | 32 +-- .../prefixallocationscheduler_controller.go | 48 ++-- controllers/ipam/suite_test.go | 2 + .../networking/aliasprefix_controller.go | 8 +- .../networking/networkinterface_controller.go | 2 +- .../networkinterface_controller_test.go | 2 +- docs/api-reference/ipam.md | 212 ++++-------------- envtestutils/apiserver/apiserver.go | 13 +- generated/openapi/api_violations.report | 2 - generated/openapi/zz_generated.openapi.go | 136 ++--------- main.go | 1 + .../ipam/prefix/storage/tableconvertor.go | 13 +- .../storage/tableconvertor.go | 11 +- 28 files changed, 357 insertions(+), 1113 deletions(-) delete mode 100644 apis/ipam/readiness.go delete mode 100644 apis/ipam/v1alpha1/readiness.go diff --git a/apis/ipam/prefix_types.go b/apis/ipam/prefix_types.go index f88df53c6..c9afb562c 100644 --- a/apis/ipam/prefix_types.go +++ b/apis/ipam/prefix_types.go @@ -46,26 +46,25 @@ func (s *PrefixSpec) IsRoot() bool { // PrefixStatus defines the observed state of Prefix type PrefixStatus struct { - // Conditions is a list of conditions of a Prefix. - Conditions []PrefixCondition + // Phase is the PrefixPhase of the Prefix. + Phase PrefixPhase + // LastPhaseTransitionTime is the last time the Phase changed values. + LastPhaseTransitionTime *metav1.Time + // Used is a list of used prefixes. Used []commonv1alpha1.IPPrefix } -type PrefixConditionType string +// PrefixPhase is a phase a Prefix can be in. +type PrefixPhase string const ( - PrefixReady PrefixConditionType = "Ready" + // PrefixPhasePending marks a prefix as waiting for allocation. + PrefixPhasePending PrefixPhase = "Pending" + // PrefixPhaseAllocated marks a prefix as allocated. + PrefixPhaseAllocated PrefixPhase = "Allocated" ) -type PrefixCondition struct { - Type PrefixConditionType - Status corev1.ConditionStatus - Reason string - Message string - LastTransitionTime metav1.Time -} - // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object // +genclient diff --git a/apis/ipam/prefixallocation_types.go b/apis/ipam/prefixallocation_types.go index c2f71f895..132488c17 100644 --- a/apis/ipam/prefixallocation_types.go +++ b/apis/ipam/prefixallocation_types.go @@ -38,26 +38,27 @@ type PrefixAllocationSpec struct { PrefixSelector *metav1.LabelSelector } +// PrefixAllocationPhase is a phase a PrefixAllocation can be in. +type PrefixAllocationPhase string + +const ( + // PrefixAllocationPhasePending marks a PrefixAllocation as waiting for allocation. + PrefixAllocationPhasePending PrefixAllocationPhase = "Pending" + // PrefixAllocationPhaseAllocated marks a PrefixAllocation as allocated by a Prefix. + PrefixAllocationPhaseAllocated PrefixAllocationPhase = "Allocated" + // PrefixAllocationPhaseFailed marks a PrefixAllocation as failed. + PrefixAllocationPhaseFailed PrefixAllocationPhase = "Failed" +) + // PrefixAllocationStatus is the status of a PrefixAllocation. type PrefixAllocationStatus struct { // Prefix is the allocated prefix, if any Prefix *commonv1alpha1.IPPrefix - // Conditions represent various state aspects of a PrefixAllocation. - Conditions []PrefixAllocationCondition -} - -type PrefixAllocationConditionType string - -const ( - PrefixAllocationReady PrefixAllocationConditionType = "Ready" -) + // LastPhaseTransitionTime is the last time the Phase changed values. + LastPhaseTransitionTime *metav1.Time -type PrefixAllocationCondition struct { - Type PrefixAllocationConditionType - Status corev1.ConditionStatus - Reason string - Message string - LastTransitionTime metav1.Time + // Phase is the phase of the PrefixAllocation. + Phase PrefixAllocationPhase } // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object diff --git a/apis/ipam/readiness.go b/apis/ipam/readiness.go deleted file mode 100644 index 751f83d52..000000000 --- a/apis/ipam/readiness.go +++ /dev/null @@ -1,100 +0,0 @@ -// Copyright 2022 OnMetal 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 ipam - -import ( - "github.com/onmetal/controller-utils/conditionutils" - corev1 "k8s.io/api/core/v1" -) - -type Readiness string - -const ( - ReadinessFailed Readiness = "Failed" - ReadinessUnknown Readiness = "Unknown" - ReadinessPending Readiness = "Pending" - ReadinessSucceeded Readiness = "Succeeded" -) - -const ( - ReasonPending = "Pending" - ReasonFailed = "Failed" -) - -var readinessToInt = map[Readiness]int{ - ReadinessFailed: -1, - ReadinessUnknown: 0, - ReadinessPending: 1, - ReadinessSucceeded: 2, -} - -func (r1 Readiness) Compare(r2 Readiness) int { - return readinessToInt[r1] - readinessToInt[r2] -} - -// Terminal reports whether the Readiness is a terminal / final Readiness that cannot be changed from. -func (r1 Readiness) Terminal() bool { - switch r1 { - case ReadinessSucceeded, ReadinessFailed: - return true - default: - return false - } -} - -func ReadinessFromStatusAndReason(status corev1.ConditionStatus, reason string) Readiness { - switch { - case status == corev1.ConditionTrue: - return ReadinessSucceeded - case status == corev1.ConditionFalse && reason == ReasonPending: - return ReadinessPending - case status == corev1.ConditionFalse && reason == ReasonFailed: - return ReadinessFailed - default: - return ReadinessUnknown - } -} - -func GetPrefixAllocationConditionsReadinessAndIndex(conditions []PrefixAllocationCondition) (Readiness, int) { - idx := conditionutils.MustFindSliceIndex(conditions, string(PrefixAllocationReady)) - if idx < 0 { - return ReadinessUnknown, idx - } - - cond := &conditions[idx] - readiness := ReadinessFromStatusAndReason(cond.Status, cond.Reason) - return readiness, idx -} - -func GetPrefixAllocationReadiness(prefixAllocation *PrefixAllocation) Readiness { - readiness, _ := GetPrefixAllocationConditionsReadinessAndIndex(prefixAllocation.Status.Conditions) - return readiness -} - -func GetPrefixConditionsReadinessAndIndex(conditions []PrefixCondition) (Readiness, int) { - idx := conditionutils.MustFindSliceIndex(conditions, string(PrefixAllocationReady)) - if idx < 0 { - return ReadinessUnknown, idx - } - - cond := &conditions[idx] - readiness := ReadinessFromStatusAndReason(cond.Status, cond.Reason) - return readiness, idx -} - -func GetPrefixReadiness(prefix *Prefix) Readiness { - readiness, _ := GetPrefixConditionsReadinessAndIndex(prefix.Status.Conditions) - return readiness -} diff --git a/apis/ipam/v1alpha1/prefix_types.go b/apis/ipam/v1alpha1/prefix_types.go index 1bf00b802..53244d8a9 100644 --- a/apis/ipam/v1alpha1/prefix_types.go +++ b/apis/ipam/v1alpha1/prefix_types.go @@ -44,32 +44,27 @@ type PrefixSpec struct { ParentSelector *metav1.LabelSelector `json:"parentSelector,omitempty"` } -func (s *PrefixSpec) IsRoot() bool { - return s.ParentRef == nil && s.ParentSelector == nil -} - // PrefixStatus defines the observed state of Prefix type PrefixStatus struct { - // Conditions is a list of conditions of a Prefix. - Conditions []PrefixCondition `json:"conditions,omitempty"` + // Phase is the PrefixPhase of the Prefix. + Phase PrefixPhase `json:"phase,omitempty"` + // LastPhaseTransitionTime is the last time the Phase changed values. + LastPhaseTransitionTime *metav1.Time `json:"lastPhaseTransitionTime,omitempty"` + // Used is a list of used prefixes. Used []commonv1alpha1.IPPrefix `json:"used,omitempty"` } -type PrefixConditionType string +// PrefixPhase is a phase a Prefix can be in. +type PrefixPhase string const ( - PrefixReady PrefixConditionType = "Ready" + // PrefixPhasePending marks a prefix as waiting for allocation. + PrefixPhasePending PrefixPhase = "Pending" + // PrefixPhaseAllocated marks a prefix as allocated. + PrefixPhaseAllocated PrefixPhase = "Allocated" ) -type PrefixCondition struct { - Type PrefixConditionType `json:"type"` - Status corev1.ConditionStatus `json:"status"` - Reason string `json:"reason,omitempty"` - Message string `json:"message,omitempty"` - LastTransitionTime metav1.Time `json:"lastTransitionTime,omitempty"` -} - // +genclient // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object @@ -82,15 +77,6 @@ type Prefix struct { Status PrefixStatus `json:"status,omitempty"` } -func (p *Prefix) IsRoot() bool { - return p.Spec.IsRoot() -} - -func (p *Prefix) Readiness() Readiness { - readiness, _ := GetPrefixConditionsReadinessAndIndex(p.Status.Conditions) - return readiness -} - // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object // PrefixList contains a list of Prefix diff --git a/apis/ipam/v1alpha1/prefixallocation_types.go b/apis/ipam/v1alpha1/prefixallocation_types.go index 86a716049..a356da2a2 100644 --- a/apis/ipam/v1alpha1/prefixallocation_types.go +++ b/apis/ipam/v1alpha1/prefixallocation_types.go @@ -38,26 +38,36 @@ type PrefixAllocationSpec struct { PrefixSelector *metav1.LabelSelector `json:"prefixSelector,omitempty"` } -// PrefixAllocationStatus is the status of a PrefixAllocation. -type PrefixAllocationStatus struct { - // Prefix is the allocated prefix, if any - Prefix *commonv1alpha1.IPPrefix `json:"prefix,omitempty"` - // Conditions represent various state aspects of a PrefixAllocation. - Conditions []PrefixAllocationCondition `json:"conditions,omitempty"` -} +// PrefixAllocationPhase is a phase a PrefixAllocation can be in. +type PrefixAllocationPhase string -type PrefixAllocationConditionType string +func (p PrefixAllocationPhase) IsTerminal() bool { + switch p { + case PrefixAllocationPhaseFailed, PrefixAllocationPhaseAllocated: + return true + default: + return false + } +} const ( - PrefixAllocationReady PrefixAllocationConditionType = "Ready" + // PrefixAllocationPhasePending marks a PrefixAllocation as waiting for allocation. + PrefixAllocationPhasePending PrefixAllocationPhase = "Pending" + // PrefixAllocationPhaseAllocated marks a PrefixAllocation as allocated by a Prefix. + PrefixAllocationPhaseAllocated PrefixAllocationPhase = "Allocated" + // PrefixAllocationPhaseFailed marks a PrefixAllocation as failed. + PrefixAllocationPhaseFailed PrefixAllocationPhase = "Failed" ) -type PrefixAllocationCondition struct { - Type PrefixAllocationConditionType `json:"type"` - Status corev1.ConditionStatus `json:"status"` - Reason string `json:"reason,omitempty"` - Message string `json:"message,omitempty"` - LastTransitionTime metav1.Time `json:"lastTransitionTime,omitempty"` +// PrefixAllocationStatus is the status of a PrefixAllocation. +type PrefixAllocationStatus struct { + // Prefix is the allocated prefix, if any + Prefix *commonv1alpha1.IPPrefix `json:"prefix,omitempty"` + + // Phase is the phase of the PrefixAllocation. + Phase PrefixAllocationPhase `json:"phase,omitempty"` + // LastPhaseTransitionTime is the last time the Phase changed values. + LastPhaseTransitionTime *metav1.Time `json:"lastPhaseTransitionTime,omitempty"` } // +genclient diff --git a/apis/ipam/v1alpha1/readiness.go b/apis/ipam/v1alpha1/readiness.go deleted file mode 100644 index 298d4d063..000000000 --- a/apis/ipam/v1alpha1/readiness.go +++ /dev/null @@ -1,101 +0,0 @@ -// Copyright 2022 OnMetal 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 v1alpha1 - -import ( - "github.com/onmetal/controller-utils/conditionutils" - corev1 "k8s.io/api/core/v1" -) - -type Readiness string - -const ( - ReadinessFailed Readiness = "Failed" - ReadinessUnknown Readiness = "Unknown" - ReadinessPending Readiness = "Pending" - ReadinessSucceeded Readiness = "Succeeded" -) - -const ( - ReasonPending = "Pending" - ReasonFailed = "Failed" - ReasonSucceeded = "Allocated" -) - -var readinessToInt = map[Readiness]int{ - ReadinessFailed: -1, - ReadinessUnknown: 0, - ReadinessPending: 1, - ReadinessSucceeded: 2, -} - -func (r1 Readiness) Compare(r2 Readiness) int { - return readinessToInt[r1] - readinessToInt[r2] -} - -// IsTerminal reports whether the Readiness is a terminal / final Readiness that cannot be changed from. -func (r1 Readiness) IsTerminal() bool { - switch r1 { - case ReadinessSucceeded, ReadinessFailed: - return true - default: - return false - } -} - -func ReadinessFromStatusAndReason(status corev1.ConditionStatus, reason string) Readiness { - switch { - case status == corev1.ConditionTrue: - return ReadinessSucceeded - case status == corev1.ConditionFalse && reason == ReasonPending: - return ReadinessPending - case status == corev1.ConditionFalse && reason == ReasonFailed: - return ReadinessFailed - default: - return ReadinessUnknown - } -} - -func GetPrefixAllocationConditionsReadinessAndIndex(conditions []PrefixAllocationCondition) (Readiness, int) { - idx := conditionutils.MustFindSliceIndex(conditions, string(PrefixAllocationReady)) - if idx < 0 { - return ReadinessUnknown, idx - } - - cond := &conditions[idx] - readiness := ReadinessFromStatusAndReason(cond.Status, cond.Reason) - return readiness, idx -} - -func GetPrefixAllocationReadiness(prefixAllocation *PrefixAllocation) Readiness { - readiness, _ := GetPrefixAllocationConditionsReadinessAndIndex(prefixAllocation.Status.Conditions) - return readiness -} - -func GetPrefixConditionsReadinessAndIndex(conditions []PrefixCondition) (Readiness, int) { - idx := conditionutils.MustFindSliceIndex(conditions, string(PrefixAllocationReady)) - if idx < 0 { - return ReadinessUnknown, idx - } - - cond := &conditions[idx] - readiness := ReadinessFromStatusAndReason(cond.Status, cond.Reason) - return readiness, idx -} - -func GetPrefixReadiness(prefix *Prefix) Readiness { - readiness, _ := GetPrefixConditionsReadinessAndIndex(prefix.Status.Conditions) - return readiness -} diff --git a/apis/ipam/v1alpha1/zz_generated.conversion.go b/apis/ipam/v1alpha1/zz_generated.conversion.go index b531a2851..0e3f56df2 100644 --- a/apis/ipam/v1alpha1/zz_generated.conversion.go +++ b/apis/ipam/v1alpha1/zz_generated.conversion.go @@ -58,16 +58,6 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddGeneratedConversionFunc((*PrefixAllocationCondition)(nil), (*ipam.PrefixAllocationCondition)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1alpha1_PrefixAllocationCondition_To_ipam_PrefixAllocationCondition(a.(*PrefixAllocationCondition), b.(*ipam.PrefixAllocationCondition), scope) - }); err != nil { - return err - } - if err := s.AddGeneratedConversionFunc((*ipam.PrefixAllocationCondition)(nil), (*PrefixAllocationCondition)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_ipam_PrefixAllocationCondition_To_v1alpha1_PrefixAllocationCondition(a.(*ipam.PrefixAllocationCondition), b.(*PrefixAllocationCondition), scope) - }); err != nil { - return err - } if err := s.AddGeneratedConversionFunc((*PrefixAllocationList)(nil), (*ipam.PrefixAllocationList)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha1_PrefixAllocationList_To_ipam_PrefixAllocationList(a.(*PrefixAllocationList), b.(*ipam.PrefixAllocationList), scope) }); err != nil { @@ -98,16 +88,6 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddGeneratedConversionFunc((*PrefixCondition)(nil), (*ipam.PrefixCondition)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1alpha1_PrefixCondition_To_ipam_PrefixCondition(a.(*PrefixCondition), b.(*ipam.PrefixCondition), scope) - }); err != nil { - return err - } - if err := s.AddGeneratedConversionFunc((*ipam.PrefixCondition)(nil), (*PrefixCondition)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_ipam_PrefixCondition_To_v1alpha1_PrefixCondition(a.(*ipam.PrefixCondition), b.(*PrefixCondition), scope) - }); err != nil { - return err - } if err := s.AddGeneratedConversionFunc((*PrefixList)(nil), (*ipam.PrefixList)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha1_PrefixList_To_ipam_PrefixList(a.(*PrefixList), b.(*ipam.PrefixList), scope) }); err != nil { @@ -215,37 +195,19 @@ func Convert_ipam_PrefixAllocation_To_v1alpha1_PrefixAllocation(in *ipam.PrefixA return autoConvert_ipam_PrefixAllocation_To_v1alpha1_PrefixAllocation(in, out, s) } -func autoConvert_v1alpha1_PrefixAllocationCondition_To_ipam_PrefixAllocationCondition(in *PrefixAllocationCondition, out *ipam.PrefixAllocationCondition, s conversion.Scope) error { - out.Type = ipam.PrefixAllocationConditionType(in.Type) - out.Status = v1.ConditionStatus(in.Status) - out.Reason = in.Reason - out.Message = in.Message - out.LastTransitionTime = in.LastTransitionTime - return nil -} - -// Convert_v1alpha1_PrefixAllocationCondition_To_ipam_PrefixAllocationCondition is an autogenerated conversion function. -func Convert_v1alpha1_PrefixAllocationCondition_To_ipam_PrefixAllocationCondition(in *PrefixAllocationCondition, out *ipam.PrefixAllocationCondition, s conversion.Scope) error { - return autoConvert_v1alpha1_PrefixAllocationCondition_To_ipam_PrefixAllocationCondition(in, out, s) -} - -func autoConvert_ipam_PrefixAllocationCondition_To_v1alpha1_PrefixAllocationCondition(in *ipam.PrefixAllocationCondition, out *PrefixAllocationCondition, s conversion.Scope) error { - out.Type = PrefixAllocationConditionType(in.Type) - out.Status = v1.ConditionStatus(in.Status) - out.Reason = in.Reason - out.Message = in.Message - out.LastTransitionTime = in.LastTransitionTime - return nil -} - -// Convert_ipam_PrefixAllocationCondition_To_v1alpha1_PrefixAllocationCondition is an autogenerated conversion function. -func Convert_ipam_PrefixAllocationCondition_To_v1alpha1_PrefixAllocationCondition(in *ipam.PrefixAllocationCondition, out *PrefixAllocationCondition, s conversion.Scope) error { - return autoConvert_ipam_PrefixAllocationCondition_To_v1alpha1_PrefixAllocationCondition(in, out, s) -} - func autoConvert_v1alpha1_PrefixAllocationList_To_ipam_PrefixAllocationList(in *PrefixAllocationList, out *ipam.PrefixAllocationList, s conversion.Scope) error { out.ListMeta = in.ListMeta - out.Items = *(*[]ipam.PrefixAllocation)(unsafe.Pointer(&in.Items)) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]ipam.PrefixAllocation, len(*in)) + for i := range *in { + if err := Convert_v1alpha1_PrefixAllocation_To_ipam_PrefixAllocation(&(*in)[i], &(*out)[i], s); err != nil { + return err + } + } + } else { + out.Items = nil + } return nil } @@ -256,7 +218,17 @@ func Convert_v1alpha1_PrefixAllocationList_To_ipam_PrefixAllocationList(in *Pref func autoConvert_ipam_PrefixAllocationList_To_v1alpha1_PrefixAllocationList(in *ipam.PrefixAllocationList, out *PrefixAllocationList, s conversion.Scope) error { out.ListMeta = in.ListMeta - out.Items = *(*[]PrefixAllocation)(unsafe.Pointer(&in.Items)) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]PrefixAllocation, len(*in)) + for i := range *in { + if err := Convert_ipam_PrefixAllocation_To_v1alpha1_PrefixAllocation(&(*in)[i], &(*out)[i], s); err != nil { + return err + } + } + } else { + out.Items = nil + } return nil } @@ -295,7 +267,8 @@ func Convert_ipam_PrefixAllocationSpec_To_v1alpha1_PrefixAllocationSpec(in *ipam func autoConvert_v1alpha1_PrefixAllocationStatus_To_ipam_PrefixAllocationStatus(in *PrefixAllocationStatus, out *ipam.PrefixAllocationStatus, s conversion.Scope) error { out.Prefix = (*commonv1alpha1.IPPrefix)(unsafe.Pointer(in.Prefix)) - out.Conditions = *(*[]ipam.PrefixAllocationCondition)(unsafe.Pointer(&in.Conditions)) + out.Phase = ipam.PrefixAllocationPhase(in.Phase) + out.LastPhaseTransitionTime = (*metav1.Time)(unsafe.Pointer(in.LastPhaseTransitionTime)) return nil } @@ -306,7 +279,8 @@ func Convert_v1alpha1_PrefixAllocationStatus_To_ipam_PrefixAllocationStatus(in * func autoConvert_ipam_PrefixAllocationStatus_To_v1alpha1_PrefixAllocationStatus(in *ipam.PrefixAllocationStatus, out *PrefixAllocationStatus, s conversion.Scope) error { out.Prefix = (*commonv1alpha1.IPPrefix)(unsafe.Pointer(in.Prefix)) - out.Conditions = *(*[]PrefixAllocationCondition)(unsafe.Pointer(&in.Conditions)) + out.LastPhaseTransitionTime = (*metav1.Time)(unsafe.Pointer(in.LastPhaseTransitionTime)) + out.Phase = PrefixAllocationPhase(in.Phase) return nil } @@ -315,34 +289,6 @@ func Convert_ipam_PrefixAllocationStatus_To_v1alpha1_PrefixAllocationStatus(in * return autoConvert_ipam_PrefixAllocationStatus_To_v1alpha1_PrefixAllocationStatus(in, out, s) } -func autoConvert_v1alpha1_PrefixCondition_To_ipam_PrefixCondition(in *PrefixCondition, out *ipam.PrefixCondition, s conversion.Scope) error { - out.Type = ipam.PrefixConditionType(in.Type) - out.Status = v1.ConditionStatus(in.Status) - out.Reason = in.Reason - out.Message = in.Message - out.LastTransitionTime = in.LastTransitionTime - return nil -} - -// Convert_v1alpha1_PrefixCondition_To_ipam_PrefixCondition is an autogenerated conversion function. -func Convert_v1alpha1_PrefixCondition_To_ipam_PrefixCondition(in *PrefixCondition, out *ipam.PrefixCondition, s conversion.Scope) error { - return autoConvert_v1alpha1_PrefixCondition_To_ipam_PrefixCondition(in, out, s) -} - -func autoConvert_ipam_PrefixCondition_To_v1alpha1_PrefixCondition(in *ipam.PrefixCondition, out *PrefixCondition, s conversion.Scope) error { - out.Type = PrefixConditionType(in.Type) - out.Status = v1.ConditionStatus(in.Status) - out.Reason = in.Reason - out.Message = in.Message - out.LastTransitionTime = in.LastTransitionTime - return nil -} - -// Convert_ipam_PrefixCondition_To_v1alpha1_PrefixCondition is an autogenerated conversion function. -func Convert_ipam_PrefixCondition_To_v1alpha1_PrefixCondition(in *ipam.PrefixCondition, out *PrefixCondition, s conversion.Scope) error { - return autoConvert_ipam_PrefixCondition_To_v1alpha1_PrefixCondition(in, out, s) -} - func autoConvert_v1alpha1_PrefixList_To_ipam_PrefixList(in *PrefixList, out *ipam.PrefixList, s conversion.Scope) error { out.ListMeta = in.ListMeta out.Items = *(*[]ipam.Prefix)(unsafe.Pointer(&in.Items)) @@ -394,7 +340,8 @@ func Convert_ipam_PrefixSpec_To_v1alpha1_PrefixSpec(in *ipam.PrefixSpec, out *Pr } func autoConvert_v1alpha1_PrefixStatus_To_ipam_PrefixStatus(in *PrefixStatus, out *ipam.PrefixStatus, s conversion.Scope) error { - out.Conditions = *(*[]ipam.PrefixCondition)(unsafe.Pointer(&in.Conditions)) + out.Phase = ipam.PrefixPhase(in.Phase) + out.LastPhaseTransitionTime = (*metav1.Time)(unsafe.Pointer(in.LastPhaseTransitionTime)) out.Used = *(*[]commonv1alpha1.IPPrefix)(unsafe.Pointer(&in.Used)) return nil } @@ -405,7 +352,8 @@ func Convert_v1alpha1_PrefixStatus_To_ipam_PrefixStatus(in *PrefixStatus, out *i } func autoConvert_ipam_PrefixStatus_To_v1alpha1_PrefixStatus(in *ipam.PrefixStatus, out *PrefixStatus, s conversion.Scope) error { - out.Conditions = *(*[]PrefixCondition)(unsafe.Pointer(&in.Conditions)) + out.Phase = PrefixPhase(in.Phase) + out.LastPhaseTransitionTime = (*metav1.Time)(unsafe.Pointer(in.LastPhaseTransitionTime)) out.Used = *(*[]commonv1alpha1.IPPrefix)(unsafe.Pointer(&in.Used)) return nil } diff --git a/apis/ipam/v1alpha1/zz_generated.deepcopy.go b/apis/ipam/v1alpha1/zz_generated.deepcopy.go index 8abd4e8eb..2604a7b1a 100644 --- a/apis/ipam/v1alpha1/zz_generated.deepcopy.go +++ b/apis/ipam/v1alpha1/zz_generated.deepcopy.go @@ -83,23 +83,6 @@ func (in *PrefixAllocation) DeepCopyObject() runtime.Object { return nil } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *PrefixAllocationCondition) DeepCopyInto(out *PrefixAllocationCondition) { - *out = *in - in.LastTransitionTime.DeepCopyInto(&out.LastTransitionTime) - return -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PrefixAllocationCondition. -func (in *PrefixAllocationCondition) DeepCopy() *PrefixAllocationCondition { - if in == nil { - return nil - } - out := new(PrefixAllocationCondition) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PrefixAllocationList) DeepCopyInto(out *PrefixAllocationList) { *out = *in @@ -170,12 +153,9 @@ func (in *PrefixAllocationStatus) DeepCopyInto(out *PrefixAllocationStatus) { in, out := &in.Prefix, &out.Prefix *out = (*in).DeepCopy() } - if in.Conditions != nil { - in, out := &in.Conditions, &out.Conditions - *out = make([]PrefixAllocationCondition, len(*in)) - for i := range *in { - (*in)[i].DeepCopyInto(&(*out)[i]) - } + if in.LastPhaseTransitionTime != nil { + in, out := &in.LastPhaseTransitionTime, &out.LastPhaseTransitionTime + *out = (*in).DeepCopy() } return } @@ -190,23 +170,6 @@ func (in *PrefixAllocationStatus) DeepCopy() *PrefixAllocationStatus { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *PrefixCondition) DeepCopyInto(out *PrefixCondition) { - *out = *in - in.LastTransitionTime.DeepCopyInto(&out.LastTransitionTime) - return -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PrefixCondition. -func (in *PrefixCondition) DeepCopy() *PrefixCondition { - if in == nil { - return nil - } - out := new(PrefixCondition) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PrefixList) DeepCopyInto(out *PrefixList) { *out = *in @@ -273,12 +236,9 @@ func (in *PrefixSpec) DeepCopy() *PrefixSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PrefixStatus) DeepCopyInto(out *PrefixStatus) { *out = *in - if in.Conditions != nil { - in, out := &in.Conditions, &out.Conditions - *out = make([]PrefixCondition, len(*in)) - for i := range *in { - (*in)[i].DeepCopyInto(&(*out)[i]) - } + if in.LastPhaseTransitionTime != nil { + in, out := &in.LastPhaseTransitionTime, &out.LastPhaseTransitionTime + *out = (*in).DeepCopy() } if in.Used != nil { in, out := &in.Used, &out.Used diff --git a/apis/ipam/validation/prefix.go b/apis/ipam/validation/prefix.go index 42f0639e5..c025a99be 100644 --- a/apis/ipam/validation/prefix.go +++ b/apis/ipam/validation/prefix.go @@ -145,9 +145,9 @@ func validatePrefixSpecUpdate(newSpec, oldSpec *ipam.PrefixSpec, fldPath *field. func ValidatePrefixStatus(status *ipam.PrefixStatus, fldPath *field.Path) field.ErrorList { var allErrs field.ErrorList - readiness, _ := ipam.GetPrefixConditionsReadinessAndIndex(status.Conditions) - switch readiness { - case ipam.ReadinessSucceeded: + phase := status.Phase + switch phase { + case ipam.PrefixPhaseAllocated: var bldr netaddr.IPSetBuilder var ( @@ -192,26 +192,20 @@ func ValidatePrefixStatusUpdate(newPrefix, oldPrefix *ipam.Prefix) field.ErrorLi allErrs = append(allErrs, apivalidation.ValidateObjectMetaAccessorUpdate(newPrefix, oldPrefix, field.NewPath("metadata"))...) allErrs = append(allErrs, ValidatePrefixStatus(&newPrefix.Status, statusField)...) - conditionsField := statusField.Child("conditions") + newPhase := newPrefix.Status.Phase + oldPhase := oldPrefix.Status.Phase - newReadiness, newReadyIdx := ipam.GetPrefixConditionsReadinessAndIndex(newPrefix.Status.Conditions) - oldReadiness, _ := ipam.GetPrefixConditionsReadinessAndIndex(oldPrefix.Status.Conditions) - - if oldReadiness.Terminal() && oldReadiness != newReadiness { - if newReadyIdx < 0 { - allErrs = append(allErrs, field.Required(conditionsField.Index(0), "terminal ready condition is missing")) - } else { - allErrs = append(allErrs, field.Forbidden(conditionsField.Index(newReadyIdx), "may not change terminal ready condition")) - } + if oldPhase == ipam.PrefixPhaseAllocated && oldPhase != newPhase { + allErrs = append(allErrs, field.Forbidden(field.NewPath("status", "phase"), "may not update from allocated to non-allocated phase")) } - if newReadiness == ipam.ReadinessSucceeded { + if newPhase == ipam.PrefixPhaseAllocated { if !newPrefix.Spec.Prefix.IsValid() { - allErrs = append(allErrs, field.Forbidden(conditionsField.Index(newReadyIdx), "spec.prefix has to be defined")) + allErrs = append(allErrs, field.Required(field.NewPath("spec", "prefix"), "must specify prefix if allocated")) } if newPrefix.Spec.ParentSelector != nil && newPrefix.Spec.ParentRef == nil { - allErrs = append(allErrs, field.Forbidden(conditionsField.Index(newReadyIdx), "child prefix cannot be allocated without parentRef set")) + allErrs = append(allErrs, field.Required(field.NewPath("spec", "parentRef"), "must specify parentRef when allocating sub-prefix")) } } diff --git a/apis/ipam/validation/prefix_test.go b/apis/ipam/validation/prefix_test.go index e7bde9e46..23fe0c3eb 100644 --- a/apis/ipam/validation/prefix_test.go +++ b/apis/ipam/validation/prefix_test.go @@ -268,26 +268,16 @@ var _ = Describe("Prefix", func() { errList := ValidatePrefixStatus(status, field.NewPath("status")) Expect(errList).To(match) }, - Entry("succeeded invalid used", + Entry("allocated invalid used", &ipam.PrefixStatus{ - Conditions: []ipam.PrefixCondition{ - { - Type: ipam.PrefixReady, - Status: corev1.ConditionTrue, - }, - }, - Used: []commonv1alpha1.IPPrefix{{}}, + Phase: ipam.PrefixPhaseAllocated, + Used: []commonv1alpha1.IPPrefix{{}}, }, ContainElement(InvalidField("status.used[0]")), ), - Entry("succeeded overlapping used", + Entry("allocated overlapping used", &ipam.PrefixStatus{ - Conditions: []ipam.PrefixCondition{ - { - Type: ipam.PrefixReady, - Status: corev1.ConditionTrue, - }, - }, + Phase: ipam.PrefixPhaseAllocated, Used: []commonv1alpha1.IPPrefix{ commonv1alpha1.MustParseIPPrefix("10.0.0.0/8"), commonv1alpha1.MustParseIPPrefix("10.0.0.0/16"), @@ -295,14 +285,9 @@ var _ = Describe("Prefix", func() { }, ContainElement(InvalidField("status.used")), ), - Entry("succeeded different ip families used", + Entry("allocated different ip families used", &ipam.PrefixStatus{ - Conditions: []ipam.PrefixCondition{ - { - Type: ipam.PrefixReady, - Status: corev1.ConditionTrue, - }, - }, + Phase: ipam.PrefixPhaseAllocated, Used: []commonv1alpha1.IPPrefix{ commonv1alpha1.MustParseIPPrefix("10.0.0.0/8"), commonv1alpha1.MustParseIPPrefix("beef::/64"), @@ -310,20 +295,15 @@ var _ = Describe("Prefix", func() { }, ContainElement(InvalidField("status.used")), ), - Entry("not succeeded but used", + Entry("not allocated but used", &ipam.PrefixStatus{ Used: []commonv1alpha1.IPPrefix{{}}, }, ContainElement(InvalidField("status.used")), ), - Entry("succeeded valid", + Entry("allocated valid", &ipam.PrefixStatus{ - Conditions: []ipam.PrefixCondition{ - { - Type: ipam.PrefixReady, - Status: corev1.ConditionTrue, - }, - }, + Phase: ipam.PrefixPhaseAllocated, Used: []commonv1alpha1.IPPrefix{ commonv1alpha1.MustParseIPPrefix("10.0.0.0/8"), commonv1alpha1.MustParseIPPrefix("11.0.0.0/8"), @@ -331,7 +311,7 @@ var _ = Describe("Prefix", func() { }, BeEmpty(), ), - Entry("not succeeded valid", + Entry("not allocated valid", &ipam.PrefixStatus{}, BeEmpty(), ), @@ -346,69 +326,43 @@ var _ = Describe("Prefix", func() { &ipam.Prefix{}, &ipam.Prefix{ Status: ipam.PrefixStatus{ - Conditions: []ipam.PrefixCondition{ - { - Type: ipam.PrefixReady, - Status: corev1.ConditionTrue, - }, - }, + Phase: ipam.PrefixPhaseAllocated, }, }, - ContainElement(RequiredField("status.conditions[0]")), + ContainElement(ForbiddenField("status.phase")), ), Entry("terminal to non-terminal ready condition", &ipam.Prefix{ Status: ipam.PrefixStatus{ - Conditions: []ipam.PrefixCondition{ - { - Type: ipam.PrefixReady, - Status: corev1.ConditionFalse, - Reason: ipam.ReasonPending, - }, - }, + Phase: ipam.PrefixPhasePending, }, }, &ipam.Prefix{ Status: ipam.PrefixStatus{ - Conditions: []ipam.PrefixCondition{ - { - Type: ipam.PrefixReady, - Status: corev1.ConditionTrue, - }, - }, + Phase: ipam.PrefixPhaseAllocated, }, }, - ContainElement(ForbiddenField("status.conditions[0]")), + ContainElement(ForbiddenField("status.phase")), ), Entry("non-terminal to terminal ready condition", &ipam.Prefix{ Status: ipam.PrefixStatus{ - Conditions: []ipam.PrefixCondition{ - { - Type: ipam.PrefixReady, - Status: corev1.ConditionTrue, - }, - }, + Phase: ipam.PrefixPhaseAllocated, }, }, &ipam.Prefix{}, - Not(ContainElement(InvalidField("status.conditions[0]"))), + Not(ContainElement(InvalidField("status.phase"))), ), - Entry("empty to succeeded but prefix not valid", + Entry("empty to allocated but prefix not valid", &ipam.Prefix{ Status: ipam.PrefixStatus{ - Conditions: []ipam.PrefixCondition{ - { - Type: ipam.PrefixReady, - Status: corev1.ConditionTrue, - }, - }, + Phase: ipam.PrefixPhaseAllocated, }, }, &ipam.Prefix{}, - ContainElement(ForbiddenField("status.conditions[0]")), + ContainElement(RequiredField("spec.prefix")), ), - Entry("empty to succeeded and prefix valid but child and no parent", + Entry("empty to allocated and prefix valid but child and no parent", &ipam.Prefix{ Spec: ipam.PrefixSpec{ Prefix: commonv1alpha1.MustParseNewIPPrefix("10.0.0.0/8"), @@ -417,12 +371,7 @@ var _ = Describe("Prefix", func() { }, }, Status: ipam.PrefixStatus{ - Conditions: []ipam.PrefixCondition{ - { - Type: ipam.PrefixReady, - Status: corev1.ConditionTrue, - }, - }, + Phase: ipam.PrefixPhaseAllocated, }, }, &ipam.Prefix{ @@ -433,20 +382,15 @@ var _ = Describe("Prefix", func() { }, }, }, - ContainElement(ForbiddenField("status.conditions[0]")), + ContainElement(RequiredField("spec.parentRef")), ), - Entry("empty to succeeded and prefix valid", + Entry("empty to allocated and prefix valid", &ipam.Prefix{ Spec: ipam.PrefixSpec{ Prefix: commonv1alpha1.MustParseNewIPPrefix("10.0.0.0/8"), }, Status: ipam.PrefixStatus{ - Conditions: []ipam.PrefixCondition{ - { - Type: ipam.PrefixReady, - Status: corev1.ConditionTrue, - }, - }, + Phase: ipam.PrefixPhaseAllocated, }, }, &ipam.Prefix{ @@ -454,7 +398,7 @@ var _ = Describe("Prefix", func() { Prefix: commonv1alpha1.MustParseNewIPPrefix("10.0.0.0/8"), }, }, - Not(ContainElement(ForbiddenField("status.conditions[0]"))), + Not(ContainElement(ForbiddenField("status.phase"))), ), Entry("used not covered by spec", &ipam.Prefix{ @@ -465,12 +409,7 @@ var _ = Describe("Prefix", func() { Used: []commonv1alpha1.IPPrefix{ commonv1alpha1.MustParseIPPrefix("11.0.0.0/8"), }, - Conditions: []ipam.PrefixCondition{ - { - Type: ipam.PrefixReady, - Status: corev1.ConditionTrue, - }, - }, + Phase: ipam.PrefixPhaseAllocated, }, }, &ipam.Prefix{ @@ -478,12 +417,7 @@ var _ = Describe("Prefix", func() { Prefix: commonv1alpha1.MustParseNewIPPrefix("10.0.0.0/8"), }, Status: ipam.PrefixStatus{ - Conditions: []ipam.PrefixCondition{ - { - Type: ipam.PrefixReady, - Status: corev1.ConditionTrue, - }, - }, + Phase: ipam.PrefixPhaseAllocated, }, }, ContainElement(ForbiddenField("status.used[0]")), @@ -494,15 +428,10 @@ var _ = Describe("Prefix", func() { Prefix: commonv1alpha1.MustParseNewIPPrefix("10.0.0.0/8"), }, Status: ipam.PrefixStatus{ + Phase: ipam.PrefixPhaseAllocated, Used: []commonv1alpha1.IPPrefix{ commonv1alpha1.MustParseIPPrefix("10.0.0.0/8"), }, - Conditions: []ipam.PrefixCondition{ - { - Type: ipam.PrefixReady, - Status: corev1.ConditionTrue, - }, - }, }, }, &ipam.Prefix{ @@ -510,12 +439,7 @@ var _ = Describe("Prefix", func() { Prefix: commonv1alpha1.MustParseNewIPPrefix("10.0.0.0/8"), }, Status: ipam.PrefixStatus{ - Conditions: []ipam.PrefixCondition{ - { - Type: ipam.PrefixReady, - Status: corev1.ConditionTrue, - }, - }, + Phase: ipam.PrefixPhaseAllocated, }, }, Not(ContainElement(ForbiddenField("status.used[0]"))), diff --git a/apis/ipam/validation/prefixallocation.go b/apis/ipam/validation/prefixallocation.go index fe48f63cd..863544420 100644 --- a/apis/ipam/validation/prefixallocation.go +++ b/apis/ipam/validation/prefixallocation.go @@ -103,15 +103,14 @@ func ValidatePrefixAllocationStatus(status *ipam.PrefixAllocationStatus, fldPath allErrs = append(allErrs, commonv1alpha1validation.ValidateIPPrefix(status.Prefix.IP().Family(), *status.Prefix, fldPath.Child("prefix"))...) } - readiness, _ := ipam.GetPrefixAllocationConditionsReadinessAndIndex(status.Conditions) - switch readiness { - case ipam.ReadinessSucceeded: + switch status.Phase { + case ipam.PrefixAllocationPhaseAllocated: if status.Prefix == nil { - allErrs = append(allErrs, field.Required(fldPath.Child("prefix"), "must specify prefix when succeeded")) + allErrs = append(allErrs, field.Required(fldPath.Child("prefix"), "must specify prefix when allocated")) } default: if status.Prefix != nil { - allErrs = append(allErrs, field.Forbidden(fldPath.Child("prefix"), "must not specify a prefix when not succeeded")) + allErrs = append(allErrs, field.Forbidden(fldPath.Child("prefix"), "must not specify a prefix when not allocated")) } } @@ -126,16 +125,10 @@ func ValidatePrefixAllocationStatusUpdate(newPrefixAllocation, oldPrefixAllocati allErrs = append(allErrs, apivalidation.ValidateObjectMetaAccessorUpdate(newPrefixAllocation, oldPrefixAllocation, field.NewPath("metadata"))...) allErrs = append(allErrs, ValidatePrefixAllocationStatus(&newPrefixAllocation.Status, statusField)...) - conditionsField := statusField.Child("conditions") - - newReadiness, newReadyIdx := ipam.GetPrefixAllocationConditionsReadinessAndIndex(newPrefixAllocation.Status.Conditions) - oldReadiness, _ := ipam.GetPrefixAllocationConditionsReadinessAndIndex(oldPrefixAllocation.Status.Conditions) - if oldReadiness.Terminal() && newReadiness != oldReadiness { - if newReadyIdx < 0 { - allErrs = append(allErrs, field.Required(conditionsField.Index(0), "terminal ready condition is missing")) - } else { - allErrs = append(allErrs, field.Forbidden(conditionsField.Index(newReadyIdx), "may not change terminal ready condition")) - } + newPhase := newPrefixAllocation.Status.Phase + oldPhase := oldPrefixAllocation.Status.Phase + if (oldPhase == ipam.PrefixAllocationPhaseFailed || oldPhase == ipam.PrefixAllocationPhaseAllocated) && newPhase != oldPhase { + allErrs = append(allErrs, field.Forbidden(field.NewPath("status", "phase"), "must not set failed / allocated allocation to non-failed / non-allocated")) } statusPrefixField := statusField.Child("prefix") diff --git a/apis/ipam/validation/prefixallocation_test.go b/apis/ipam/validation/prefixallocation_test.go index bf67c4d40..36eed7c90 100644 --- a/apis/ipam/validation/prefixallocation_test.go +++ b/apis/ipam/validation/prefixallocation_test.go @@ -189,32 +189,22 @@ var _ = Describe("PrefixAllocation", func() { errList := ValidatePrefixAllocationStatus(status, field.NewPath("status")) Expect(errList).To(match) }, - Entry("succeeded but no result", + Entry("allocated but no result", &ipam.PrefixAllocationStatus{ - Conditions: []ipam.PrefixAllocationCondition{ - { - Type: ipam.PrefixAllocationReady, - Status: corev1.ConditionTrue, - }, - }, + Phase: ipam.PrefixAllocationPhaseAllocated, }, ContainElement(RequiredField("status.prefix")), ), - Entry("not succeeded but result", + Entry("not allocated but result", &ipam.PrefixAllocationStatus{ Prefix: commonv1alpha1.MustParseNewIPPrefix("10.0.0.0/8"), }, ContainElement(ForbiddenField("status.prefix")), ), - Entry("succeeded with result", + Entry("allocated with result", &ipam.PrefixAllocationStatus{ Prefix: commonv1alpha1.MustParseNewIPPrefix("10.0.0.0/8"), - Conditions: []ipam.PrefixAllocationCondition{ - { - Type: ipam.PrefixAllocationReady, - Status: corev1.ConditionTrue, - }, - }, + Phase: ipam.PrefixAllocationPhaseAllocated, }, Not(ContainElement(ForbiddenField("status.prefix"))), ), @@ -225,44 +215,27 @@ var _ = Describe("PrefixAllocation", func() { errList := ValidatePrefixAllocationStatusUpdate(newPrefixAllocation, oldPrefixAllocation) Expect(errList).To(match) }, - Entry("update from terminal readiness to missing readiness", + Entry("update from terminal phase to missing phase", &ipam.PrefixAllocation{}, &ipam.PrefixAllocation{ Status: ipam.PrefixAllocationStatus{ - Conditions: []ipam.PrefixAllocationCondition{ - { - Type: ipam.PrefixAllocationReady, - Status: corev1.ConditionFalse, - Reason: ipam.ReasonFailed, - }, - }, + Phase: ipam.PrefixAllocationPhaseFailed, }, }, - ContainElement(RequiredField("status.conditions[0]")), + ContainElement(ForbiddenField("status.phase")), ), - Entry("update from terminal readiness to other readiness", + Entry("update from terminal phase to other phase", &ipam.PrefixAllocation{ Status: ipam.PrefixAllocationStatus{ - Conditions: []ipam.PrefixAllocationCondition{ - { - Type: ipam.PrefixAllocationReady, - Status: corev1.ConditionTrue, - }, - }, + Phase: ipam.PrefixAllocationPhaseAllocated, }, }, &ipam.PrefixAllocation{ Status: ipam.PrefixAllocationStatus{ - Conditions: []ipam.PrefixAllocationCondition{ - { - Type: ipam.PrefixAllocationReady, - Status: corev1.ConditionFalse, - Reason: ipam.ReasonFailed, - }, - }, + Phase: ipam.PrefixAllocationPhaseFailed, }, }, - ContainElement(ForbiddenField("status.conditions[0]")), + ContainElement(ForbiddenField("status.phase")), ), Entry("prefix mismatch with spec.prefix", &ipam.PrefixAllocation{ @@ -271,13 +244,8 @@ var _ = Describe("PrefixAllocation", func() { PrefixRef: &corev1.LocalObjectReference{Name: "foo"}, }, Status: ipam.PrefixAllocationStatus{ + Phase: ipam.PrefixAllocationPhaseAllocated, Prefix: commonv1alpha1.MustParseNewIPPrefix("10.0.0.9/32"), - Conditions: []ipam.PrefixAllocationCondition{ - { - Type: ipam.PrefixAllocationReady, - Status: corev1.ConditionTrue, - }, - }, }, }, &ipam.PrefixAllocation{ @@ -296,13 +264,8 @@ var _ = Describe("PrefixAllocation", func() { PrefixRef: &corev1.LocalObjectReference{Name: "foo"}, }, Status: ipam.PrefixAllocationStatus{ + Phase: ipam.PrefixAllocationPhaseAllocated, Prefix: commonv1alpha1.MustParseNewIPPrefix("10.0.0.9/32"), - Conditions: []ipam.PrefixAllocationCondition{ - { - Type: ipam.PrefixAllocationReady, - Status: corev1.ConditionTrue, - }, - }, }, }, &ipam.PrefixAllocation{ @@ -322,13 +285,8 @@ var _ = Describe("PrefixAllocation", func() { PrefixRef: &corev1.LocalObjectReference{Name: "foo"}, }, Status: ipam.PrefixAllocationStatus{ + Phase: ipam.PrefixAllocationPhaseAllocated, Prefix: commonv1alpha1.MustParseNewIPPrefix("beef::/8"), - Conditions: []ipam.PrefixAllocationCondition{ - { - Type: ipam.PrefixAllocationReady, - Status: corev1.ConditionTrue, - }, - }, }, }, &ipam.PrefixAllocation{ @@ -346,13 +304,8 @@ var _ = Describe("PrefixAllocation", func() { PrefixLength: 32, }, Status: ipam.PrefixAllocationStatus{ + Phase: ipam.PrefixAllocationPhaseAllocated, Prefix: commonv1alpha1.MustParseNewIPPrefix("10.0.0.9/32"), - Conditions: []ipam.PrefixAllocationCondition{ - { - Type: ipam.PrefixAllocationReady, - Status: corev1.ConditionTrue, - }, - }, }, }, &ipam.PrefixAllocation{ diff --git a/apis/ipam/zz_generated.deepcopy.go b/apis/ipam/zz_generated.deepcopy.go index 15d4a670d..1d9ef4900 100644 --- a/apis/ipam/zz_generated.deepcopy.go +++ b/apis/ipam/zz_generated.deepcopy.go @@ -83,23 +83,6 @@ func (in *PrefixAllocation) DeepCopyObject() runtime.Object { return nil } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *PrefixAllocationCondition) DeepCopyInto(out *PrefixAllocationCondition) { - *out = *in - in.LastTransitionTime.DeepCopyInto(&out.LastTransitionTime) - return -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PrefixAllocationCondition. -func (in *PrefixAllocationCondition) DeepCopy() *PrefixAllocationCondition { - if in == nil { - return nil - } - out := new(PrefixAllocationCondition) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PrefixAllocationList) DeepCopyInto(out *PrefixAllocationList) { *out = *in @@ -170,12 +153,9 @@ func (in *PrefixAllocationStatus) DeepCopyInto(out *PrefixAllocationStatus) { in, out := &in.Prefix, &out.Prefix *out = (*in).DeepCopy() } - if in.Conditions != nil { - in, out := &in.Conditions, &out.Conditions - *out = make([]PrefixAllocationCondition, len(*in)) - for i := range *in { - (*in)[i].DeepCopyInto(&(*out)[i]) - } + if in.LastPhaseTransitionTime != nil { + in, out := &in.LastPhaseTransitionTime, &out.LastPhaseTransitionTime + *out = (*in).DeepCopy() } return } @@ -190,23 +170,6 @@ func (in *PrefixAllocationStatus) DeepCopy() *PrefixAllocationStatus { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *PrefixCondition) DeepCopyInto(out *PrefixCondition) { - *out = *in - in.LastTransitionTime.DeepCopyInto(&out.LastTransitionTime) - return -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PrefixCondition. -func (in *PrefixCondition) DeepCopy() *PrefixCondition { - if in == nil { - return nil - } - out := new(PrefixCondition) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PrefixList) DeepCopyInto(out *PrefixList) { *out = *in @@ -273,12 +236,9 @@ func (in *PrefixSpec) DeepCopy() *PrefixSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PrefixStatus) DeepCopyInto(out *PrefixStatus) { *out = *in - if in.Conditions != nil { - in, out := &in.Conditions, &out.Conditions - *out = make([]PrefixCondition, len(*in)) - for i := range *in { - (*in)[i].DeepCopyInto(&(*out)[i]) - } + if in.LastPhaseTransitionTime != nil { + in, out := &in.LastPhaseTransitionTime, &out.LastPhaseTransitionTime + *out = (*in).DeepCopy() } if in.Used != nil { in, out := &in.Used, &out.Used diff --git a/controllers/ipam/ipam.go b/controllers/ipam/ipam.go index cd498e79e..f24544c4e 100644 --- a/controllers/ipam/ipam.go +++ b/controllers/ipam/ipam.go @@ -18,8 +18,6 @@ import ( "context" ipamv1alpha1 "github.com/onmetal/onmetal-api/apis/ipam/v1alpha1" - "k8s.io/apimachinery/pkg/api/meta" - "k8s.io/apimachinery/pkg/runtime" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -35,19 +33,3 @@ func SetupPrefixSpecIPFamilyFieldIndexer(mgr ctrl.Manager) error { return []string{string(prefix.Spec.IPFamily)} }) } - -type readerClient struct { - client.Reader - noReaderClient -} - -type noReaderClient interface { - client.Writer - client.StatusClient - Scheme() *runtime.Scheme - RESTMapper() meta.RESTMapper -} - -func ReaderClient(reader client.Reader, c client.Client) client.Client { - return readerClient{reader, c} -} diff --git a/controllers/ipam/prefix_controller.go b/controllers/ipam/prefix_controller.go index 0eb33f21b..ba49f6f85 100644 --- a/controllers/ipam/prefix_controller.go +++ b/controllers/ipam/prefix_controller.go @@ -25,12 +25,10 @@ import ( "github.com/go-logr/logr" "github.com/onmetal/controller-utils/clientutils" - "github.com/onmetal/controller-utils/conditionutils" commonv1alpha1 "github.com/onmetal/onmetal-api/apis/common/v1alpha1" ipamv1alpha1 "github.com/onmetal/onmetal-api/apis/ipam/v1alpha1" "github.com/onmetal/onmetal-api/equality" "inet.af/netaddr" - corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" @@ -152,7 +150,7 @@ func (r *PrefixReconciler) delete(ctx context.Context, log logr.Logger, prefix * continue } - if ipamv1alpha1.GetPrefixAllocationReadiness(&allocation) != ipamv1alpha1.ReadinessSucceeded { + if allocation.Status.Phase != ipamv1alpha1.PrefixAllocationPhaseAllocated { continue } @@ -173,13 +171,17 @@ func (r *PrefixReconciler) delete(ctx context.Context, log logr.Logger, prefix * return ctrl.Result{}, nil } -func (r *PrefixReconciler) patchStatus(ctx context.Context, prefix *ipamv1alpha1.Prefix, readyCond ipamv1alpha1.PrefixCondition) error { +func (r *PrefixReconciler) patchStatus(ctx context.Context, prefix *ipamv1alpha1.Prefix, phase ipamv1alpha1.PrefixPhase) error { + now := metav1.Now() base := prefix.DeepCopy() - conditionutils.MustUpdateSlice(&prefix.Status.Conditions, string(ipamv1alpha1.PrefixReady), - conditionutils.UpdateFromCondition{Condition: readyCond}, - ) + + if prefix.Status.Phase != phase { + prefix.Status.LastPhaseTransitionTime = &now + } + prefix.Status.Phase = phase + if err := r.Status().Patch(ctx, prefix, client.MergeFrom(base)); err != nil { - return fmt.Errorf("error patching ready condition: %w", err) + return fmt.Errorf("error patching status: %w", err) } return nil } @@ -201,7 +203,7 @@ func (r *PrefixReconciler) allocationMatchesPrefix(allocation *ipamv1alpha1.Pref return (p1 == nil) == (p2 == nil) && (p1 == nil || *p1 == *p2) } - if ipamv1alpha1.GetPrefixAllocationReadiness(allocation) == ipamv1alpha1.ReadinessSucceeded { + if allocation.Status.Phase == ipamv1alpha1.PrefixAllocationPhaseAllocated { switch { case prefix.Spec.Prefix.IsValid(): return *prefix.Spec.Prefix == *allocation.Status.Prefix @@ -222,10 +224,16 @@ func (r *PrefixReconciler) allocationMatchesPrefix(allocation *ipamv1alpha1.Pref } } -// prefixAllocationLess determines if allocation is less than other by first comparing the readiness of both conditions -// and then, if both readiness states are the same, prefers the older object. +var prefixAllocationPhaseValue = map[ipamv1alpha1.PrefixAllocationPhase]int{ + ipamv1alpha1.PrefixAllocationPhaseAllocated: 2, + ipamv1alpha1.PrefixAllocationPhaseFailed: -1, + ipamv1alpha1.PrefixAllocationPhasePending: 1, +} + +// prefixAllocationLess determines if allocation is less than other by first comparing the phase of both +// and then, if both phases are the same, prefers the older object. func (r *PrefixReconciler) prefixAllocationLess(allocation, other *ipamv1alpha1.PrefixAllocation) bool { - return ipamv1alpha1.GetPrefixAllocationReadiness(allocation) < ipamv1alpha1.GetPrefixAllocationReadiness(other) || + return prefixAllocationPhaseValue[allocation.Status.Phase] < prefixAllocationPhaseValue[other.Status.Phase] || allocation.GetCreationTimestamp().Time.After(other.GetCreationTimestamp().Time) } @@ -284,7 +292,7 @@ func (r *PrefixReconciler) findActiveAllocation(prefix *ipamv1alpha1.Prefix, all func (r *PrefixReconciler) allocateSubPrefix(ctx context.Context, log logr.Logger, prefix *ipamv1alpha1.Prefix) (*ipamv1alpha1.PrefixAllocation, time.Duration, error) { log.V(1).Info("Listing prefix allocations controlled by prefix") allocationList := &ipamv1alpha1.PrefixAllocationList{} - if err := clientutils.ListAndFilterControlledBy(ctx, ReaderClient(r.APIReader, r.Client), prefix, allocationList, + if err := clientutils.ListAndFilterControlledBy(ctx, clientutils.ReaderClient(r.APIReader, r.Client), prefix, allocationList, client.InNamespace(prefix.Namespace), client.MatchingLabels{ prefixAllocationRequesterUIDLabel: string(prefix.UID), @@ -303,13 +311,14 @@ func (r *PrefixReconciler) allocateSubPrefix(ctx context.Context, log logr.Logge return created, 0, err } - readiness := r.adjustedAllocationReadiness(prefix, active) - log.V(1).Info("Found active allocation", "Active", active.Name, "Readiness", readiness) + allocationPhase := r.adjustedAllocationPhase(active) switch { - case readiness == ipamv1alpha1.ReadinessSucceeded: + case allocationPhase == ipamv1alpha1.PrefixAllocationPhaseAllocated: + log.V(1).Info("Allocation is allocated") r.forgetAllocationBackoffFor(client.ObjectKeyFromObject(prefix)) return active, 0, nil - case readiness == ipamv1alpha1.ReadinessFailed: + case allocationPhase == ipamv1alpha1.PrefixAllocationPhaseFailed: + log.V(1).Info("Allocation is failed") retry, err := r.canRetryAllocation(ctx, prefix, active) if err != nil || !retry { return active, 0, err @@ -322,45 +331,43 @@ func (r *PrefixReconciler) allocateSubPrefix(ctx context.Context, log logr.Logge } // Delete / free up the old allocation + log.V(1).Info("Deleting old allocation") if err := r.Delete(ctx, active); client.IgnoreNotFound(err) != nil { return nil, 0, err } // Actually initiate the retry + log.V(1).Info("Retrying allocation") created, err := r.createNewAllocation(ctx, prefix) return created, 0, err default: + log.V(1).Info("Allocation is not allocated / failed", "Phase", allocationPhase) return active, 0, nil } } -// adjustedAllocationReadiness calculates an adjusted readiness of a PrefixAllocation. -// For the adjusted readiness, it is considered -// * If the allocation is in a terminal state, that state is returned. +// adjustedAllocationPhase calculates an adjusted phase of a PrefixAllocation. +// For the adjusted phase, it is considered +// * If the allocation is in a terminal phase, that state is returned. // * If the allocation is not scheduled, that state is returned. +// * If the allocation is scheduled but no lastTransitionTime has been recorded, that state is returned // * If the allocation is in a non-terminal state, and it has been scheduled, once a configurable timeout has passed, // it is considered to be failed. -func (r *PrefixReconciler) adjustedAllocationReadiness(prefix *ipamv1alpha1.Prefix, allocation *ipamv1alpha1.PrefixAllocation) ipamv1alpha1.Readiness { - allocationReadiness, allocationReadyIdx := ipamv1alpha1.GetPrefixAllocationConditionsReadinessAndIndex(allocation.Status.Conditions) - if allocationReadiness.IsTerminal() || allocation.Spec.PrefixRef == nil { - return allocationReadiness +func (r *PrefixReconciler) adjustedAllocationPhase(allocation *ipamv1alpha1.PrefixAllocation) ipamv1alpha1.PrefixAllocationPhase { + allocationPhase := allocation.Status.Phase + if allocationPhase.IsTerminal() || allocation.Spec.PrefixRef == nil { + return allocationPhase } - now := time.Now() - lastTransitionTime := now - if allocationReadyIdx != -1 { - lastTransitionTime = allocation.Status.Conditions[allocationReadyIdx].LastTransitionTime.Time - } else { - _, idx := ipamv1alpha1.GetPrefixConditionsReadinessAndIndex(prefix.Status.Conditions) - if idx != -1 { - lastTransitionTime = prefix.Status.Conditions[idx].LastTransitionTime.Time - } + lastTransitionTime := allocation.Status.LastPhaseTransitionTime + if lastTransitionTime.IsZero() { + return allocationPhase } - if lastTransitionTime.Add(r.PrefixAllocationTimeout).After(now) { - return ipamv1alpha1.ReadinessFailed + if lastTransitionTime.Add(r.PrefixAllocationTimeout).After(time.Now()) { + return ipamv1alpha1.PrefixAllocationPhaseFailed } - return allocationReadiness + return allocationPhase } func (r *PrefixReconciler) createNewAllocation(ctx context.Context, prefix *ipamv1alpha1.Prefix) (*ipamv1alpha1.PrefixAllocation, error) { @@ -405,16 +412,12 @@ func (r *PrefixReconciler) pruneOutdatedAllocations(ctx context.Context, log log func (r *PrefixReconciler) allocateSelf(ctx context.Context, log logr.Logger, prefix *ipamv1alpha1.Prefix) (ok bool, backoff time.Duration, err error) { switch { - case prefix.Readiness() == ipamv1alpha1.ReadinessSucceeded: + case prefix.Status.Phase == ipamv1alpha1.PrefixPhaseAllocated: log.V(1).Info("Prefix is allocated") return true, 0, nil - case prefix.IsRoot(): - log.V(1).Info("Patching root prefix as succeeded") - if err := r.patchStatus(ctx, prefix, ipamv1alpha1.PrefixCondition{ - Status: corev1.ConditionTrue, - Reason: ipamv1alpha1.ReasonSucceeded, - Message: "Root prefixes are allocated by default.", - }); err != nil { + case prefix.Spec.ParentRef == nil && prefix.Spec.ParentSelector == nil: + log.V(1).Info("Marking root prefix as allocated") + if err := r.patchStatus(ctx, prefix, ipamv1alpha1.PrefixPhaseAllocated); err != nil { return false, 0, fmt.Errorf("error marking root prefix as allocated: %w", err) } return false, 0, nil @@ -422,39 +425,28 @@ func (r *PrefixReconciler) allocateSelf(ctx context.Context, log logr.Logger, pr log.V(1).Info("Allocating sub-prefix") allocation, backoff, err := r.allocateSubPrefix(ctx, log, prefix) if err != nil { - if err := r.patchStatus(ctx, prefix, ipamv1alpha1.PrefixCondition{ - Status: corev1.ConditionUnknown, - Reason: "ErrorAllocating", - Message: fmt.Sprintf("Allocating resulted in an error: %v.", err), - }); err != nil { - log.Error(err, "Error patching readiness") + log.Error(err, "Error allocating sub-prefix") + if err := r.patchStatus(ctx, prefix, ipamv1alpha1.PrefixPhasePending); err != nil { + log.Error(err, "Error patching status") } return false, 0, fmt.Errorf("error applying allocation: %w", err) } - readiness := ipamv1alpha1.GetPrefixAllocationReadiness(allocation) + allocationPhase := allocation.Status.Phase switch { - case readiness == ipamv1alpha1.ReadinessSucceeded && !prefix.Spec.Prefix.IsValid(): + case allocationPhase == ipamv1alpha1.PrefixAllocationPhaseAllocated && !prefix.Spec.Prefix.IsValid(): if err := r.assignPrefix(ctx, prefix, allocation); err != nil { return false, 0, fmt.Errorf("error patching prefix assignment: %w", err) } return false, 0, nil - case readiness == ipamv1alpha1.ReadinessSucceeded: + case allocationPhase == ipamv1alpha1.PrefixAllocationPhaseAllocated: log.V(1).Info("Marking sub prefix as allocated") - if err := r.patchStatus(ctx, prefix, ipamv1alpha1.PrefixCondition{ - Status: corev1.ConditionTrue, - Reason: ipamv1alpha1.ReasonSucceeded, - Message: "Prefix has been successfully allocated.", - }); err != nil { + if err := r.patchStatus(ctx, prefix, ipamv1alpha1.PrefixPhaseAllocated); err != nil { return false, 0, fmt.Errorf("error marking prefix as allocated: %w", err) } return false, 0, nil default: - if err := r.patchStatus(ctx, prefix, ipamv1alpha1.PrefixCondition{ - Status: corev1.ConditionFalse, - Reason: ipamv1alpha1.ReasonPending, - Message: "Prefix is not yet allocated.", - }); err != nil { + if err := r.patchStatus(ctx, prefix, ipamv1alpha1.PrefixPhasePending); err != nil { return false, backoff, fmt.Errorf("error marking prefix as pending: %w", err) } return false, backoff, nil @@ -488,12 +480,12 @@ func (r *PrefixReconciler) processAllocations(ctx context.Context, log logr.Logg ) availableBuilder.AddPrefix(prefix.Spec.Prefix.IPPrefix) for _, allocation := range list.Items { - readiness := ipamv1alpha1.GetPrefixAllocationReadiness(&allocation) + allocationPhase := allocation.Status.Phase switch { - case readiness == ipamv1alpha1.ReadinessSucceeded: + case allocationPhase == ipamv1alpha1.PrefixAllocationPhaseAllocated: used = append(used, *allocation.Status.Prefix) availableBuilder.RemovePrefix(allocation.Status.Prefix.IPPrefix) - case readiness != ipamv1alpha1.ReadinessFailed: + case allocationPhase != ipamv1alpha1.PrefixAllocationPhaseFailed: newAllocations = append(newAllocations, allocation) } } @@ -525,13 +517,16 @@ func (r *PrefixReconciler) patchAllocationStatus( ctx context.Context, allocation *ipamv1alpha1.PrefixAllocation, prefix *commonv1alpha1.IPPrefix, - readyCond ipamv1alpha1.PrefixAllocationCondition, + phase ipamv1alpha1.PrefixAllocationPhase, ) error { + now := metav1.Now() base := allocation.DeepCopy() + allocation.Status.Prefix = prefix - conditionutils.MustUpdateSlice(&allocation.Status.Conditions, string(ipamv1alpha1.PrefixAllocationReady), - conditionutils.UpdateFromCondition{Condition: readyCond}, - ) + if allocation.Status.Phase != phase { + allocation.Status.LastPhaseTransitionTime = &now + } + allocation.Status.Phase = phase if err := r.Status().Patch(ctx, allocation, client.MergeFrom(base)); err != nil { return fmt.Errorf("error updating allocation status: %w", err) } @@ -548,31 +543,19 @@ func (r *PrefixReconciler) processAllocation(ctx context.Context, log logr.Logge switch { case !ok && terminal: log.V(1).Info("Marking terminally non-allocatable allocation as failed") - if err := r.patchAllocationStatus(ctx, allocation, allocation.Status.Prefix, ipamv1alpha1.PrefixAllocationCondition{ - Status: corev1.ConditionFalse, - Reason: ipamv1alpha1.ReasonFailed, - Message: "The prefix can not fit the allocation.", - }); client.IgnoreNotFound(err) != nil { + if err := r.patchAllocationStatus(ctx, allocation, allocation.Status.Prefix, ipamv1alpha1.PrefixAllocationPhaseFailed); client.IgnoreNotFound(err) != nil { return available, netaddr.IPPrefix{}, fmt.Errorf("could not mark allocation as failed: %w", err) } return available, netaddr.IPPrefix{}, nil case !ok: log.V(1).Info("Marking non-allocatable allocation as pending") - if err := r.patchAllocationStatus(ctx, allocation, allocation.Status.Prefix, ipamv1alpha1.PrefixAllocationCondition{ - Status: corev1.ConditionFalse, - Reason: ipamv1alpha1.ReasonPending, - Message: "Could not allocate request.", - }); client.IgnoreNotFound(err) != nil { + if err := r.patchAllocationStatus(ctx, allocation, allocation.Status.Prefix, ipamv1alpha1.PrefixAllocationPhasePending); client.IgnoreNotFound(err) != nil { return available, netaddr.IPPrefix{}, fmt.Errorf("could not mark allocation as pending: %w", err) } return available, netaddr.IPPrefix{}, nil default: log.V(1).Info("Marking allocation as allocated") - if err := r.patchAllocationStatus(ctx, allocation, commonv1alpha1.NewIPPrefix(res), ipamv1alpha1.PrefixAllocationCondition{ - Status: corev1.ConditionTrue, - Reason: ipamv1alpha1.ReasonSucceeded, - Message: "The request has been successfully allocated.", - }); err != nil { + if err := r.patchAllocationStatus(ctx, allocation, commonv1alpha1.NewIPPrefix(res), ipamv1alpha1.PrefixAllocationPhaseAllocated); err != nil { return available, netaddr.IPPrefix{}, fmt.Errorf("error marking allocation as succeeded: %w", err) } return newAvailableSet, res, nil @@ -684,7 +667,7 @@ func (r *PrefixReconciler) enqueueByAllocationPrefixRef() handler.EventHandler { func (r *PrefixReconciler) enqueueByPrefixParentSelector(ctx context.Context, log logr.Logger) handler.EventHandler { return handler.EnqueueRequestsFromMapFunc(func(obj client.Object) []ctrl.Request { prefix := obj.(*ipamv1alpha1.Prefix) - if !isPrefixReadyAndNotDeleting(prefix) { + if !isPrefixAllocatedAndNotDeleting(prefix) { return nil } @@ -723,7 +706,7 @@ func (r *PrefixReconciler) enqueueByPrefixParentSelector(ctx context.Context, lo func (r *PrefixReconciler) enqueueByPrefixParentRef(ctx context.Context, log logr.Logger) handler.EventHandler { return handler.EnqueueRequestsFromMapFunc(func(obj client.Object) []ctrl.Request { prefix := obj.(*ipamv1alpha1.Prefix) - if !isPrefixReadyAndNotDeleting(prefix) { + if !isPrefixAllocatedAndNotDeleting(prefix) { return nil } diff --git a/controllers/ipam/prefix_controller_test.go b/controllers/ipam/prefix_controller_test.go index fc3618cf5..b3d43ed72 100644 --- a/controllers/ipam/prefix_controller_test.go +++ b/controllers/ipam/prefix_controller_test.go @@ -56,7 +56,7 @@ var _ = Describe("PrefixReconciler", func() { prefixKey := client.ObjectKeyFromObject(prefix) Eventually(func(g Gomega) { Expect(k8sClient.Get(ctx, prefixKey, prefix)).To(Succeed()) - g.Expect(ipamv1alpha1.GetPrefixReadiness(prefix)).To(Equal(ipamv1alpha1.ReadinessSucceeded)) + g.Expect(prefix.Status.Phase).To(Equal(ipamv1alpha1.PrefixPhaseAllocated)) g.Expect(prefix.Status.Used).To(BeEmpty()) }).Should(Succeed()) }) @@ -96,7 +96,7 @@ var _ = Describe("PrefixReconciler", func() { childPrefixKey := client.ObjectKeyFromObject(childPrefix) Eventually(func(g Gomega) { Expect(k8sClient.Get(ctx, childPrefixKey, childPrefix)).To(Succeed()) - g.Expect(ipamv1alpha1.GetPrefixReadiness(childPrefix)).To(Equal(ipamv1alpha1.ReadinessSucceeded)) + g.Expect(childPrefix.Status.Phase).To(Equal(ipamv1alpha1.PrefixPhaseAllocated)) g.Expect(childPrefix.Spec.ParentRef).To(Equal(&corev1.LocalObjectReference{ Name: rootPrefix.Name, })) @@ -113,7 +113,7 @@ var _ = Describe("PrefixReconciler", func() { Expect(clientutils.ListAndFilterControlledBy(ctx, k8sClient, childPrefix, list, client.InNamespace(ns.Name))).To(Succeed()) Expect(list.Items).To(HaveLen(1)) allocation := list.Items[0] - Expect(ipamv1alpha1.GetPrefixAllocationReadiness(&allocation)).To(Equal(ipamv1alpha1.ReadinessSucceeded)) + Expect(allocation.Status.Phase).To(Equal(ipamv1alpha1.PrefixAllocationPhaseAllocated)) Expect(allocation.Spec).To(Equal(ipamv1alpha1.PrefixAllocationSpec{ IPFamily: corev1.IPv4Protocol, PrefixLength: 28, @@ -158,13 +158,13 @@ var _ = Describe("PrefixReconciler", func() { childPrefixKey := client.ObjectKeyFromObject(childPrefix) Eventually(func(g Gomega) { g.Expect(k8sClient.Get(ctx, childPrefixKey, childPrefix)).Should(Succeed()) - g.Expect(ipamv1alpha1.GetPrefixReadiness(childPrefix)).To(Equal(ipamv1alpha1.ReadinessPending)) + g.Expect(childPrefix.Status.Phase).To(Equal(ipamv1alpha1.PrefixPhasePending)) list := &ipamv1alpha1.PrefixAllocationList{} g.Expect(clientutils.ListAndFilterControlledBy(ctx, k8sClient, childPrefix, list, client.InNamespace(ns.Name))).To(Succeed()) g.Expect(list.Items).To(HaveLen(1)) allocation := list.Items[0] - g.Expect(ipamv1alpha1.GetPrefixAllocationReadiness(&allocation)).To(Equal(ipamv1alpha1.ReadinessFailed)) + g.Expect(allocation.Status.Phase).To(Equal(ipamv1alpha1.PrefixAllocationPhaseFailed)) }).Should(Succeed()) }) @@ -191,9 +191,9 @@ var _ = Describe("PrefixReconciler", func() { childPrefixKey := client.ObjectKeyFromObject(childPrefix) Consistently(func(g Gomega) { g.Expect(k8sClient.Get(ctx, childPrefixKey, childPrefix)).Should(Succeed()) - g.Expect(ipamv1alpha1.GetPrefixReadiness(childPrefix)).To(Or( - Equal(ipamv1alpha1.ReadinessPending), - Equal(ipamv1alpha1.ReadinessUnknown), + g.Expect(childPrefix.Status.Phase).To(Or( + BeEquivalentTo(""), + Equal(ipamv1alpha1.PrefixPhasePending), )) g.Expect(childPrefix.Spec.ParentRef).To(BeNil()) }).Should(Succeed()) @@ -203,7 +203,7 @@ var _ = Describe("PrefixReconciler", func() { notMatchingRootPrefix := &ipamv1alpha1.Prefix{ ObjectMeta: metav1.ObjectMeta{ Namespace: ns.Name, - GenerateName: "test-root-", + GenerateName: "not-matching-root-", }, Spec: ipamv1alpha1.PrefixSpec{ Prefix: prefixValue, @@ -214,7 +214,7 @@ var _ = Describe("PrefixReconciler", func() { By("checking that the child prefix is not being assigned") Consistently(func(g Gomega) { g.Expect(k8sClient.Get(ctx, childPrefixKey, childPrefix)).Should(Succeed()) - g.Expect(ipamv1alpha1.GetPrefixReadiness(childPrefix)).To(Equal(ipamv1alpha1.ReadinessPending)) + g.Expect(childPrefix.Status.Phase).To(Equal(ipamv1alpha1.PrefixPhasePending)) g.Expect(childPrefix.Spec.ParentRef).To(BeNil()) }).Should(Succeed()) @@ -222,7 +222,7 @@ var _ = Describe("PrefixReconciler", func() { rootPrefix := &ipamv1alpha1.Prefix{ ObjectMeta: metav1.ObjectMeta{ Namespace: ns.Name, - GenerateName: "test-root-", + GenerateName: "matching-root-", Labels: map[string]string{ "foo": "bar", }, @@ -237,7 +237,7 @@ var _ = Describe("PrefixReconciler", func() { expectedChildPrefix := commonv1alpha1.MustParseIPPrefix("10.0.0.0/28") Eventually(func(g Gomega) { Expect(k8sClient.Get(ctx, childPrefixKey, childPrefix)).To(Succeed()) - g.Expect(ipamv1alpha1.GetPrefixReadiness(childPrefix)).To(Equal(ipamv1alpha1.ReadinessSucceeded)) + g.Expect(childPrefix.Status.Phase).To(Equal(ipamv1alpha1.PrefixPhaseAllocated)) g.Expect(childPrefix.Spec.ParentRef).To(Equal(&corev1.LocalObjectReference{ Name: rootPrefix.Name, })) @@ -266,7 +266,7 @@ var _ = Describe("PrefixReconciler", func() { }, }, })) - Expect(ipamv1alpha1.GetPrefixAllocationReadiness(allocation)).To(Equal(ipamv1alpha1.ReadinessSucceeded)) + Expect(allocation.Status.Phase).To(Equal(ipamv1alpha1.PrefixAllocationPhaseAllocated)) Expect(allocation.Status.Prefix).To(HaveValue(Equal(expectedChildPrefix))) }) @@ -300,7 +300,7 @@ var _ = Describe("PrefixReconciler", func() { childPrefixKey := client.ObjectKeyFromObject(childPrefix) Eventually(func(g Gomega) { Expect(k8sClient.Get(ctx, childPrefixKey, childPrefix)).To(Succeed()) - g.Expect(ipamv1alpha1.GetPrefixReadiness(childPrefix)).To(Equal(ipamv1alpha1.ReadinessSucceeded)) + g.Expect(childPrefix.Status.Phase).To(Equal(ipamv1alpha1.PrefixPhaseAllocated)) }).Should(Succeed()) }) @@ -334,7 +334,7 @@ var _ = Describe("PrefixReconciler", func() { childPrefixKey := client.ObjectKeyFromObject(childPrefix) Consistently(func(g Gomega) { Expect(k8sClient.Get(ctx, childPrefixKey, childPrefix)).To(Succeed()) - g.Expect(ipamv1alpha1.GetPrefixReadiness(childPrefix)).NotTo(Equal(ipamv1alpha1.ReadinessSucceeded)) + g.Expect(childPrefix.Status.Phase).NotTo(Equal(ipamv1alpha1.PrefixPhaseAllocated)) }).Should(Succeed()) }) @@ -369,7 +369,7 @@ var _ = Describe("PrefixReconciler", func() { childPrefixKey := client.ObjectKeyFromObject(childPrefix) Consistently(func(g Gomega) { Expect(k8sClient.Get(ctx, childPrefixKey, childPrefix)).To(Succeed()) - g.Expect(ipamv1alpha1.GetPrefixReadiness(childPrefix)).NotTo(Equal(ipamv1alpha1.ReadinessSucceeded)) + g.Expect(childPrefix.Status.Phase).NotTo(Equal(ipamv1alpha1.PrefixPhaseAllocated)) }).Should(Succeed()) }) }) diff --git a/controllers/ipam/prefixallocationscheduler_controller.go b/controllers/ipam/prefixallocationscheduler_controller.go index 1697150a8..9e770fadb 100644 --- a/controllers/ipam/prefixallocationscheduler_controller.go +++ b/controllers/ipam/prefixallocationscheduler_controller.go @@ -20,13 +20,13 @@ import ( "math/rand" "github.com/go-logr/logr" - "github.com/onmetal/controller-utils/conditionutils" ipamv1alpha1 "github.com/onmetal/onmetal-api/apis/ipam/v1alpha1" "inet.af/netaddr" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/handler" @@ -37,6 +37,7 @@ import ( type PrefixAllocationScheduler struct { client.Client Scheme *runtime.Scheme + Events record.EventRecorder } //+kubebuilder:rbac:groups=ipam.api.onmetal.de,resources=prefixes,verbs=get;list;watch @@ -62,25 +63,12 @@ func (s *PrefixAllocationScheduler) reconcileExists(ctx context.Context, log log return s.reconcile(ctx, log, allocation) } -func (s *PrefixAllocationScheduler) patchReadiness(ctx context.Context, allocation *ipamv1alpha1.PrefixAllocation, readyCond ipamv1alpha1.PrefixAllocationCondition) error { - base := allocation.DeepCopy() - conditionutils.MustUpdateSlice(&allocation.Status.Conditions, string(ipamv1alpha1.PrefixAllocationReady), - conditionutils.UpdateStatus(readyCond.Status), - conditionutils.UpdateReason(readyCond.Reason), - conditionutils.UpdateMessage(readyCond.Message), - ) - if err := s.Status().Patch(ctx, allocation, client.MergeFrom(base)); err != nil { - return fmt.Errorf("error patching ready condition: %w", err) - } - return nil -} - -func isPrefixReadyAndNotDeleting(prefix *ipamv1alpha1.Prefix) bool { +func isPrefixAllocatedAndNotDeleting(prefix *ipamv1alpha1.Prefix) bool { return prefix.DeletionTimestamp.IsZero() && - ipamv1alpha1.GetPrefixReadiness(prefix) == ipamv1alpha1.ReadinessSucceeded + prefix.Status.Phase == ipamv1alpha1.PrefixPhaseAllocated } -func (s *PrefixAllocationScheduler) prefixRefForAllocation(ctx context.Context, log logr.Logger, allocation *ipamv1alpha1.PrefixAllocation) (string, error) { +func (s *PrefixAllocationScheduler) prefixForAllocation(ctx context.Context, log logr.Logger, allocation *ipamv1alpha1.PrefixAllocation) (string, error) { sel, err := metav1.LabelSelectorAsSelector(allocation.Spec.PrefixSelector) if err != nil { return "", fmt.Errorf("error building label selector: %w", err) @@ -97,7 +85,7 @@ func (s *PrefixAllocationScheduler) prefixRefForAllocation(ctx context.Context, var suitable []ipamv1alpha1.Prefix for _, prefix := range list.Items { - if !isPrefixReadyAndNotDeleting(&prefix) { + if !isPrefixAllocatedAndNotDeleting(&prefix) { continue } @@ -158,29 +146,23 @@ func (s *PrefixAllocationScheduler) reconcile(ctx context.Context, log logr.Logg log.V(1).Info("Allocation has no selector") return ctrl.Result{}, nil } - if readiness := ipamv1alpha1.GetPrefixAllocationReadiness(allocation); readiness.IsTerminal() { - log.V(1).Info("Allocation is in terminal state", "Readiness", readiness) + if allocation.Status.Phase.IsTerminal() { + log.V(1).Info("Allocation is in terminal state", "Phase", allocation.Status.Phase) return ctrl.Result{}, nil } - log.V(1).Info("Determining suitable prefix ref for allocation") - ref, err := s.prefixRefForAllocation(ctx, log, allocation) + log.V(1).Info("Determining suitable prefix for allocation") + ref, err := s.prefixForAllocation(ctx, log, allocation) if err != nil { - return ctrl.Result{}, fmt.Errorf("error computing prefix ref for allocation: %w", err) + return ctrl.Result{}, fmt.Errorf("error finding prefix for allocation: %w", err) } if ref == "" { - log.V(1).Info("No suitable prefix ref found") - if err := s.patchReadiness(ctx, allocation, ipamv1alpha1.PrefixAllocationCondition{ - Status: corev1.ConditionFalse, - Reason: ipamv1alpha1.ReasonPending, - Message: "There is currently no prefix that satisfies the requirements.", - }); err != nil { - return ctrl.Result{}, fmt.Errorf("error patching readiness: %w", err) - } + log.V(1).Info("No suitable prefix found") + s.Events.Event(allocation, corev1.EventTypeNormal, "NoSuitablePrefix", "No suitable prefix for scheduling allocation found.") return ctrl.Result{}, nil } - log.V(1).Info("Suitable prefix ref found, assigning allocation", "PrefixRef", ref) + log.V(1).Info("Suitable prefix found, assigning allocation", "Prefix", ref) base := allocation.DeepCopy() allocation.Spec.PrefixRef = &corev1.LocalObjectReference{Name: ref} if err := s.Patch(ctx, allocation, client.MergeFrom(base)); err != nil { @@ -217,7 +199,7 @@ func (s *PrefixAllocationScheduler) SetupWithManager(mgr manager.Manager) error func (s *PrefixAllocationScheduler) enqueueByMatchingPrefix(ctx context.Context, log logr.Logger) handler.EventHandler { return handler.EnqueueRequestsFromMapFunc(func(obj client.Object) []ctrl.Request { prefix := obj.(*ipamv1alpha1.Prefix) - if !isPrefixReadyAndNotDeleting(prefix) { + if !isPrefixAllocatedAndNotDeleting(prefix) { return nil } diff --git a/controllers/ipam/suite_test.go b/controllers/ipam/suite_test.go index a8cdb92fb..aace11334 100644 --- a/controllers/ipam/suite_test.go +++ b/controllers/ipam/suite_test.go @@ -29,6 +29,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/envtest" @@ -124,6 +125,7 @@ var _ = BeforeSuite(func() { err = (&PrefixAllocationScheduler{ Client: k8sManager.GetClient(), Scheme: k8sManager.GetScheme(), + Events: &record.FakeRecorder{}, }).SetupWithManager(k8sManager) Expect(err).NotTo(HaveOccurred()) diff --git a/controllers/networking/aliasprefix_controller.go b/controllers/networking/aliasprefix_controller.go index 9dd305ca0..bccc3612b 100644 --- a/controllers/networking/aliasprefix_controller.go +++ b/controllers/networking/aliasprefix_controller.go @@ -86,12 +86,12 @@ func (r *AliasPrefixReconciler) applyPrefix(ctx context.Context, log logr.Logger return nil, fmt.Errorf("error managing prefix: %w", err) } - readiness := ipamv1alpha1.GetPrefixReadiness(prefix) - if readiness == ipamv1alpha1.ReadinessSucceeded { - log.V(1).Info("Ephemeral prefix is ready") + phase := prefix.Status.Phase + if phase == ipamv1alpha1.PrefixPhaseAllocated { + log.V(1).Info("Ephemeral prefix is allocated") return prefix.Spec.Prefix, nil } - log.V(1).Info("Ephemeral prefix is not yet ready", "Readiness", readiness) + log.V(1).Info("Ephemeral prefix is not yet allocated", "Phase", phase) return nil, nil default: return nil, fmt.Errorf("invalid prefix source %#v", prefixSrc) diff --git a/controllers/networking/networkinterface_controller.go b/controllers/networking/networkinterface_controller.go index c59710224..78285c4ff 100644 --- a/controllers/networking/networkinterface_controller.go +++ b/controllers/networking/networkinterface_controller.go @@ -132,7 +132,7 @@ func (r *NetworkInterfaceReconciler) applyIPs(ctx context.Context, nic *networki return nil, fmt.Errorf("error managing ephemeral prefix %s: %w", prefix.Name, err) } - if ipamv1alpha1.GetPrefixReadiness(prefix) == ipamv1alpha1.ReadinessSucceeded { + if prefix.Status.Phase == ipamv1alpha1.PrefixPhaseAllocated { ips = append(ips, prefix.Spec.Prefix.IP()) } } diff --git a/controllers/networking/networkinterface_controller_test.go b/controllers/networking/networkinterface_controller_test.go index 3d9f9e8e3..5b0bdd5da 100644 --- a/controllers/networking/networkinterface_controller_test.go +++ b/controllers/networking/networkinterface_controller_test.go @@ -134,7 +134,7 @@ var _ = Describe("NetworkInterfaceReconciler", func() { ParentRef: &corev1.LocalObjectReference{Name: rootPrefix.Name}, Prefix: commonv1alpha1.MustParseNewIPPrefix("10.0.0.1/32"), })) - g.Expect(ipamv1alpha1.GetPrefixReadiness(prefix)).To(Equal(ipamv1alpha1.ReadinessSucceeded)) + g.Expect(prefix.Status.Phase).To(Equal(ipamv1alpha1.PrefixPhaseAllocated)) }).Should(Succeed()) By("waiting for the network interface to report the correct ips") diff --git a/docs/api-reference/ipam.md b/docs/api-reference/ipam.md index 34c4bd427..10b22b883 100644 --- a/docs/api-reference/ipam.md +++ b/docs/api-reference/ipam.md @@ -292,95 +292,30 @@ PrefixAllocationStatus -

PrefixAllocationCondition -

+

PrefixAllocationPhase +(string alias)

(Appears on:PrefixAllocationStatus)

+

PrefixAllocationPhase is a phase a PrefixAllocation can be in.

- + - - - + - - - - - - - - - - - - + - - - - - + - - -
FieldValue Description
-type
- - -PrefixAllocationConditionType - - +

"Allocated"

PrefixAllocationPhaseAllocated marks a PrefixAllocation as allocated by a Prefix.

 -
-status
- - -Kubernetes core/v1.ConditionStatus - - -		 -
-reason
- -string - -		 -
-message
- -string - +

"Failed"

PrefixAllocationPhaseFailed marks a PrefixAllocation as failed.

 -
-lastTransitionTime
- - -Kubernetes meta/v1.Time - - -		 +

"Pending"

PrefixAllocationPhasePending marks a PrefixAllocation as waiting for allocation.

-

PrefixAllocationConditionType -(string alias)

-

-(Appears on:PrefixAllocationCondition) -

-
-
- - - - - - - - -
ValueDescription

"Ready"

PrefixAllocationSpec @@ -496,81 +431,20 @@ github.com/onmetal/onmetal-api/apis/common/v1alpha1.IPPrefix -conditions
- - -[]PrefixAllocationCondition - - - - -

Conditions represent various state aspects of a PrefixAllocation.

- - - - -

PrefixCondition -

-

-(Appears on:PrefixStatus) -

-
-
- - - - - - - - - - - - - - - - - - - - - -
FieldDescription
-type
+phase
- -PrefixConditionType + +PrefixAllocationPhase 		+

Phase is the phase of the PrefixAllocation.
-status
- - -Kubernetes core/v1.ConditionStatus - - -		 -
-reason
- -string - -		 -
-message
- -string - -		 -
-lastTransitionTime
+lastPhaseTransitionTime
Kubernetes meta/v1.Time @@ -578,16 +452,18 @@ Kubernetes meta/v1.Time 		+

LastPhaseTransitionTime is the last time the Phase changed values.
-

PrefixConditionType +

PrefixPhase (string alias)

-(Appears on:PrefixCondition) +(Appears on:PrefixStatus)

+

PrefixPhase is a phase a Prefix can be in.

@@ -596,8 +472,12 @@ Kubernetes meta/v1.Time - - + + + +
Description

"Ready"

"Allocated"

PrefixPhaseAllocated marks a prefix as allocated.

+

"Pending"

PrefixPhasePending marks a prefix as waiting for allocation.

+

PrefixSpec @@ -702,15 +582,28 @@ Kubernetes meta/v1.LabelSelector -conditions
+phase
+ + +PrefixPhase + + + + +

Phase is the PrefixPhase of the Prefix.

+ + + + +lastPhaseTransitionTime
- -[]PrefixCondition + +Kubernetes meta/v1.Time -

Conditions is a list of conditions of a Prefix.

+

LastPhaseTransitionTime is the last time the Phase changed values.

@@ -838,27 +731,6 @@ Kubernetes meta/v1.LabelSelector -

Readiness -(string alias)

-
-
- - - - - - - - - - - - - - - - -
ValueDescription

"Failed"

"Pending"

"Succeeded"

"Unknown"

Generated with gen-crd-api-reference-docs diff --git a/envtestutils/apiserver/apiserver.go b/envtestutils/apiserver/apiserver.go index bb0ed4498..0836ee49e 100644 --- a/envtestutils/apiserver/apiserver.go +++ b/envtestutils/apiserver/apiserver.go @@ -64,8 +64,10 @@ type Options struct { Host string Port int CertDir string - Stdout io.Writer - Stderr io.Writer + + AttachOutput bool + Stdout io.Writer + Stderr io.Writer HealthTimeout time.Duration WaitTimeout time.Duration @@ -98,12 +100,19 @@ func setAPIServerOptionsDefaults(opts *Options) { if opts.WaitTimeout == 0 { opts.WaitTimeout = 20 * time.Second } + if opts.AttachOutput { + opts.Stdout = os.Stdout + opts.Stderr = os.Stderr + } } func New(cfg *rest.Config, opts Options) (*APIServer, error) { if opts.MainPath == "" && len(opts.Command) == 0 { return nil, fmt.Errorf("must specify opts.MainPath or opts.Command") } + if opts.AttachOutput && (opts.Stdout != nil || opts.Stderr != nil) { + return nil, fmt.Errorf("must not specify AttachOutput and Stdout / Stderr simultaneously") + } if len(opts.ETCDServers) == 0 { return nil, fmt.Errorf("must specify opts.ETCDServers") } diff --git a/generated/openapi/api_violations.report b/generated/openapi/api_violations.report index e05906896..53a9e9bf9 100644 --- a/generated/openapi/api_violations.report +++ b/generated/openapi/api_violations.report @@ -9,8 +9,6 @@ API rule violation: list_type_missing,github.com/onmetal/onmetal-api/apis/comput API rule violation: list_type_missing,github.com/onmetal/onmetal-api/apis/compute/v1alpha1,MachineStatus,Conditions API rule violation: list_type_missing,github.com/onmetal/onmetal-api/apis/compute/v1alpha1,MachineStatus,Interfaces API rule violation: list_type_missing,github.com/onmetal/onmetal-api/apis/compute/v1alpha1,MachineStatus,VolumeAttachments -API rule violation: list_type_missing,github.com/onmetal/onmetal-api/apis/ipam/v1alpha1,PrefixAllocationStatus,Conditions -API rule violation: list_type_missing,github.com/onmetal/onmetal-api/apis/ipam/v1alpha1,PrefixStatus,Conditions API rule violation: list_type_missing,github.com/onmetal/onmetal-api/apis/ipam/v1alpha1,PrefixStatus,Used API rule violation: list_type_missing,github.com/onmetal/onmetal-api/apis/networking/v1alpha1,AliasPrefixRouting,Destinations API rule violation: list_type_missing,github.com/onmetal/onmetal-api/apis/networking/v1alpha1,NetworkInterfaceBinding,IPs diff --git a/generated/openapi/zz_generated.openapi.go b/generated/openapi/zz_generated.openapi.go index 6fa944915..6203655d2 100644 --- a/generated/openapi/zz_generated.openapi.go +++ b/generated/openapi/zz_generated.openapi.go @@ -62,11 +62,9 @@ func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenA "github.com/onmetal/onmetal-api/apis/compute/v1alpha1.VolumeStatus": schema_onmetal_api_apis_compute_v1alpha1_VolumeStatus(ref), "github.com/onmetal/onmetal-api/apis/ipam/v1alpha1.Prefix": schema_onmetal_api_apis_ipam_v1alpha1_Prefix(ref), "github.com/onmetal/onmetal-api/apis/ipam/v1alpha1.PrefixAllocation": schema_onmetal_api_apis_ipam_v1alpha1_PrefixAllocation(ref), - "github.com/onmetal/onmetal-api/apis/ipam/v1alpha1.PrefixAllocationCondition": schema_onmetal_api_apis_ipam_v1alpha1_PrefixAllocationCondition(ref), "github.com/onmetal/onmetal-api/apis/ipam/v1alpha1.PrefixAllocationList": schema_onmetal_api_apis_ipam_v1alpha1_PrefixAllocationList(ref), "github.com/onmetal/onmetal-api/apis/ipam/v1alpha1.PrefixAllocationSpec": schema_onmetal_api_apis_ipam_v1alpha1_PrefixAllocationSpec(ref), "github.com/onmetal/onmetal-api/apis/ipam/v1alpha1.PrefixAllocationStatus": schema_onmetal_api_apis_ipam_v1alpha1_PrefixAllocationStatus(ref), - "github.com/onmetal/onmetal-api/apis/ipam/v1alpha1.PrefixCondition": schema_onmetal_api_apis_ipam_v1alpha1_PrefixCondition(ref), "github.com/onmetal/onmetal-api/apis/ipam/v1alpha1.PrefixList": schema_onmetal_api_apis_ipam_v1alpha1_PrefixList(ref), "github.com/onmetal/onmetal-api/apis/ipam/v1alpha1.PrefixSpec": schema_onmetal_api_apis_ipam_v1alpha1_PrefixSpec(ref), "github.com/onmetal/onmetal-api/apis/ipam/v1alpha1.PrefixStatus": schema_onmetal_api_apis_ipam_v1alpha1_PrefixStatus(ref), @@ -1610,53 +1608,6 @@ func schema_onmetal_api_apis_ipam_v1alpha1_PrefixAllocation(ref common.Reference } } -func schema_onmetal_api_apis_ipam_v1alpha1_PrefixAllocationCondition(ref common.ReferenceCallback) common.OpenAPIDefinition { - return common.OpenAPIDefinition{ - Schema: spec.Schema{ - SchemaProps: spec.SchemaProps{ - Type: []string{"object"}, - Properties: map[string]spec.Schema{ - "type": { - SchemaProps: spec.SchemaProps{ - Default: "", - Type: []string{"string"}, - Format: "", - }, - }, - "status": { - SchemaProps: spec.SchemaProps{ - Default: "", - Type: []string{"string"}, - Format: "", - }, - }, - "reason": { - SchemaProps: spec.SchemaProps{ - Type: []string{"string"}, - Format: "", - }, - }, - "message": { - SchemaProps: spec.SchemaProps{ - Type: []string{"string"}, - Format: "", - }, - }, - "lastTransitionTime": { - SchemaProps: spec.SchemaProps{ - Default: map[string]interface{}{}, - Ref: ref("k8s.io/apimachinery/pkg/apis/meta/v1.Time"), - }, - }, - }, - Required: []string{"type", "status"}, - }, - }, - Dependencies: []string{ - "k8s.io/apimachinery/pkg/apis/meta/v1.Time"}, - } -} - func schema_onmetal_api_apis_ipam_v1alpha1_PrefixAllocationList(ref common.ReferenceCallback) common.OpenAPIDefinition { return common.OpenAPIDefinition{ Schema: spec.Schema{ @@ -1766,72 +1717,24 @@ func schema_onmetal_api_apis_ipam_v1alpha1_PrefixAllocationStatus(ref common.Ref Ref: ref("github.com/onmetal/onmetal-api/apis/common/v1alpha1.IPPrefix"), }, }, - "conditions": { - SchemaProps: spec.SchemaProps{ - Description: "Conditions represent various state aspects of a PrefixAllocation.", - Type: []string{"array"}, - Items: &spec.SchemaOrArray{ - Schema: &spec.Schema{ - SchemaProps: spec.SchemaProps{ - Default: map[string]interface{}{}, - Ref: ref("github.com/onmetal/onmetal-api/apis/ipam/v1alpha1.PrefixAllocationCondition"), - }, - }, - }, - }, - }, - }, - }, - }, - Dependencies: []string{ - "github.com/onmetal/onmetal-api/apis/common/v1alpha1.IPPrefix", "github.com/onmetal/onmetal-api/apis/ipam/v1alpha1.PrefixAllocationCondition"}, - } -} - -func schema_onmetal_api_apis_ipam_v1alpha1_PrefixCondition(ref common.ReferenceCallback) common.OpenAPIDefinition { - return common.OpenAPIDefinition{ - Schema: spec.Schema{ - SchemaProps: spec.SchemaProps{ - Type: []string{"object"}, - Properties: map[string]spec.Schema{ - "type": { - SchemaProps: spec.SchemaProps{ - Default: "", - Type: []string{"string"}, - Format: "", - }, - }, - "status": { - SchemaProps: spec.SchemaProps{ - Default: "", - Type: []string{"string"}, - Format: "", - }, - }, - "reason": { - SchemaProps: spec.SchemaProps{ - Type: []string{"string"}, - Format: "", - }, - }, - "message": { + "phase": { SchemaProps: spec.SchemaProps{ - Type: []string{"string"}, - Format: "", + Description: "Phase is the phase of the PrefixAllocation.", + Type: []string{"string"}, + Format: "", }, }, - "lastTransitionTime": { + "lastPhaseTransitionTime": { SchemaProps: spec.SchemaProps{ - Default: map[string]interface{}{}, - Ref: ref("k8s.io/apimachinery/pkg/apis/meta/v1.Time"), + Description: "LastPhaseTransitionTime is the last time the Phase changed values.", + Ref: ref("k8s.io/apimachinery/pkg/apis/meta/v1.Time"), }, }, }, - Required: []string{"type", "status"}, }, }, Dependencies: []string{ - "k8s.io/apimachinery/pkg/apis/meta/v1.Time"}, + "github.com/onmetal/onmetal-api/apis/common/v1alpha1.IPPrefix", "k8s.io/apimachinery/pkg/apis/meta/v1.Time"}, } } @@ -1938,18 +1841,17 @@ func schema_onmetal_api_apis_ipam_v1alpha1_PrefixStatus(ref common.ReferenceCall Description: "PrefixStatus defines the observed state of Prefix", Type: []string{"object"}, Properties: map[string]spec.Schema{ - "conditions": { + "phase": { SchemaProps: spec.SchemaProps{ - Description: "Conditions is a list of conditions of a Prefix.", - Type: []string{"array"}, - Items: &spec.SchemaOrArray{ - Schema: &spec.Schema{ - SchemaProps: spec.SchemaProps{ - Default: map[string]interface{}{}, - Ref: ref("github.com/onmetal/onmetal-api/apis/ipam/v1alpha1.PrefixCondition"), - }, - }, - }, + Description: "Phase is the PrefixPhase of the Prefix.", + Type: []string{"string"}, + Format: "", + }, + }, + "lastPhaseTransitionTime": { + SchemaProps: spec.SchemaProps{ + Description: "LastPhaseTransitionTime is the last time the Phase changed values.", + Ref: ref("k8s.io/apimachinery/pkg/apis/meta/v1.Time"), }, }, "used": { @@ -1970,7 +1872,7 @@ func schema_onmetal_api_apis_ipam_v1alpha1_PrefixStatus(ref common.ReferenceCall }, }, Dependencies: []string{ - "github.com/onmetal/onmetal-api/apis/common/v1alpha1.IPPrefix", "github.com/onmetal/onmetal-api/apis/ipam/v1alpha1.PrefixCondition"}, + "github.com/onmetal/onmetal-api/apis/common/v1alpha1.IPPrefix", "k8s.io/apimachinery/pkg/apis/meta/v1.Time"}, } } diff --git a/main.go b/main.go index ef5821320..86a64a673 100644 --- a/main.go +++ b/main.go @@ -278,6 +278,7 @@ func main() { if err = (&ipamcontrollers.PrefixAllocationScheduler{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), + Events: mgr.GetEventRecorderFor("prefix-allocation-scheduler"), }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "PrefixAllocationScheduler") os.Exit(1) diff --git a/registry/ipam/prefix/storage/tableconvertor.go b/registry/ipam/prefix/storage/tableconvertor.go index 0c66fa33e..da5240416 100644 --- a/registry/ipam/prefix/storage/tableconvertor.go +++ b/registry/ipam/prefix/storage/tableconvertor.go @@ -17,7 +17,6 @@ package storage import ( "context" - "github.com/onmetal/controller-utils/conditionutils" "github.com/onmetal/onmetal-api/apis/ipam" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/api/meta/table" @@ -34,7 +33,7 @@ var ( {Name: "Name", Type: "string", Format: "name", Description: objectMetaSwaggerDoc["name"]}, {Name: "Prefix", Type: "string", Description: "The managed prefix, if any"}, {Name: "Parent", Type: "string", Description: "The parent, if any"}, - {Name: "State", Type: "string", Description: "The allocation of the prefix"}, + {Name: "Phase", Type: "string", Description: "The phase of the prefix"}, {Name: "Age", Type: "string", Format: "date", Description: objectMetaSwaggerDoc["creationTimestamp"]}, } ) @@ -43,12 +42,6 @@ func newTableConvertor() *convertor { return &convertor{} } -func prefixReadyState(prefix *ipam.Prefix) string { - readyCond := ipam.PrefixCondition{} - conditionutils.MustFindSlice(prefix.Status.Conditions, string(ipam.PrefixReady), &readyCond) - return readyCond.Reason -} - func (c *convertor) ConvertToTable(ctx context.Context, obj runtime.Object, tableOptions runtime.Object) (*metav1.Table, error) { tab := &metav1.Table{ ColumnDefinitions: headers, @@ -78,8 +71,8 @@ func (c *convertor) ConvertToTable(ctx context.Context, obj runtime.Object, tabl } else { cells = append(cells, "") } - if readyState := prefixReadyState(prefix); readyState != "" { - cells = append(cells, readyState) + if phase := prefix.Status.Phase; phase != "" { + cells = append(cells, phase) } else { cells = append(cells, "") } diff --git a/registry/ipam/prefixallocation/storage/tableconvertor.go b/registry/ipam/prefixallocation/storage/tableconvertor.go index 7dcc54ba4..1013bb2f9 100644 --- a/registry/ipam/prefixallocation/storage/tableconvertor.go +++ b/registry/ipam/prefixallocation/storage/tableconvertor.go @@ -18,7 +18,6 @@ import ( "context" "fmt" - "github.com/onmetal/controller-utils/conditionutils" "github.com/onmetal/onmetal-api/apis/ipam" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/api/meta/table" @@ -45,12 +44,6 @@ func newTableConvertor() *convertor { return &convertor{} } -func prefixAllocationReadyState(prefixAllocation *ipam.PrefixAllocation) string { - readyCond := ipam.PrefixAllocationCondition{} - conditionutils.MustFindSlice(prefixAllocation.Status.Conditions, string(ipam.PrefixAllocationReady), &readyCond) - return readyCond.Reason -} - func prefixAllocationRequest(prefixAllocation *ipam.PrefixAllocation) string { spec := prefixAllocation.Spec switch { @@ -99,8 +92,8 @@ func (c *convertor) ConvertToTable(ctx context.Context, obj runtime.Object, tabl } else { cells = append(cells, "") } - if readyState := prefixAllocationReadyState(prefixAllocation); readyState != "" { - cells = append(cells, readyState) + if phase := prefixAllocation.Status.Phase; phase != "" { + cells = append(cells, phase) } else { cells = append(cells, "") }