Skip to content

Commit

Permalink
Remove Unknown state and stop creating unknown OOBs (#56)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Gchbg
Gchbg authored Apr 29, 2024
1 parent 0964549 commit ffa72a1
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 48 deletions.
13 changes: 6 additions & 7 deletions api/v1alpha1/oob_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ type OOBStatus struct {
// +optional
FirmwareVersion string `json:"firmwareVersion,omitempty"`

// +kubebuilder:validation:Enum=Ready;Unready;Ignored;Unknown;Error
// +kubebuilder:validation:Enum=Ready;InProgress;NoEndpoint;Ignored;Error
// +optional
State OOBState `json:"state,omitempty"`

Expand All @@ -99,11 +99,11 @@ const (
type OOBState string

const (
OOBStateReady OOBState = "Ready"
OOBStateUnready OOBState = "Unready"
OOBStateIgnored OOBState = "Ignored"
OOBStateUnknown OOBState = "Unknown"
OOBStateError OOBState = "Error"
OOBStateReady OOBState = "Ready"
OOBStateInProgress OOBState = "InProgress"
OOBStateNoEndpoint OOBState = "NoEndpoint"
OOBStateIgnored OOBState = "Ignored"
OOBStateError OOBState = "Error"
)

const (
Expand All @@ -112,7 +112,6 @@ const (
OOBConditionReasonInProgress = "InProgress"
OOBConditionReasonNoEndpoint = "NoEndpoint"
OOBConditionReasonIgnored = "Ignored"
OOBConditionReasonUnknown = "Unknown"
OOBConditionReasonError = "Error"
)

Expand Down
4 changes: 2 additions & 2 deletions config/crd/bases/metal.ironcore.dev_oobs.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,9 +208,9 @@ spec:
state:
enum:
- Ready
- Unready
- InProgress
- NoEndpoint
- Ignored
- Unknown
- Error
type: string
type:
Expand Down
54 changes: 26 additions & 28 deletions internal/controller/oob_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -282,10 +282,9 @@ func (r *OOBReconciler) reconcile(ctx context.Context, oob *metalv1alpha1.OOB) (
}

ctx, advance, err = r.runPhase(ctx, oob, oobRecPhase{
name: "Credentials",
run: r.processCredentials,
errType: OOBErrorBadCredentials,
readyReasons: []string{metalv1alpha1.OOBConditionReasonUnknown},
name: "Credentials",
run: r.processCredentials,
errType: OOBErrorBadCredentials,
})
if !advance {
return ctrl.Result{}, err
Expand Down Expand Up @@ -400,7 +399,7 @@ func (r *OOBReconciler) processIgnoreAnnotation(ctx context.Context, oob *metalv
Reason: metalv1alpha1.OOBConditionReasonIgnored,
})
} else if oob.Status.State == metalv1alpha1.OOBStateIgnored {
return r.setCondition(ctx, oob, nil, nil, metalv1alpha1.OOBStateUnready, metav1.Condition{
return r.setCondition(ctx, oob, nil, nil, metalv1alpha1.OOBStateInProgress, metav1.Condition{
Type: metalv1alpha1.OOBConditionTypeReady,
Status: metav1.ConditionFalse,
Reason: metalv1alpha1.OOBConditionReasonInProgress,
Expand All @@ -426,7 +425,7 @@ func (r *OOBReconciler) processInitial(ctx context.Context, oob *metalv1alpha1.O

_, ok := ssa.GetCondition(oob.Status.Conditions, metalv1alpha1.OOBConditionTypeReady)
if oob.Status.State == "" || !ok {
return r.setCondition(ctx, oob, apply, nil, metalv1alpha1.OOBStateUnready, metav1.Condition{
return r.setCondition(ctx, oob, apply, nil, metalv1alpha1.OOBStateInProgress, metav1.Condition{
Type: metalv1alpha1.OOBConditionTypeReady,
Status: metav1.ConditionFalse,
Reason: metalv1alpha1.OOBConditionReasonInProgress,
Expand Down Expand Up @@ -511,7 +510,7 @@ func (r *OOBReconciler) processEndpoint(ctx context.Context, oob *metalv1alpha1.
apply = apply.WithSpec(util.Ensure(apply.Spec).
WithEndpointRef(*oob.Spec.EndpointRef))

ctx, apply, status, err = r.setCondition(ctx, oob, apply, status, metalv1alpha1.OOBStateUnready, metav1.Condition{
ctx, apply, status, err = r.setCondition(ctx, oob, apply, status, metalv1alpha1.OOBStateInProgress, metav1.Condition{
Type: metalv1alpha1.OOBConditionTypeReady,
Status: metav1.ConditionFalse,
Reason: metalv1alpha1.OOBConditionReasonInProgress,
Expand All @@ -523,7 +522,7 @@ func (r *OOBReconciler) processEndpoint(ctx context.Context, oob *metalv1alpha1.
break
}
if !found {
return r.setCondition(ctx, oob, apply, status, metalv1alpha1.OOBStateUnready, metav1.Condition{
return r.setCondition(ctx, oob, apply, status, metalv1alpha1.OOBStateNoEndpoint, metav1.Condition{
Type: metalv1alpha1.OOBConditionTypeReady,
Status: metav1.ConditionFalse,
Reason: metalv1alpha1.OOBConditionReasonNoEndpoint,
Expand Down Expand Up @@ -555,7 +554,7 @@ func (r *OOBReconciler) processEndpoint(ctx context.Context, oob *metalv1alpha1.
if oob.Status.State == metalv1alpha1.OOBStateError {
cond, _ := ssa.GetCondition(oob.Status.Conditions, metalv1alpha1.OOBConditionTypeReady)
if strings.HasPrefix(cond.Message, OOBErrorBadEndpoint+": ") {
return r.setCondition(ctx, oob, apply, status, metalv1alpha1.OOBStateUnready, metav1.Condition{
return r.setCondition(ctx, oob, apply, status, metalv1alpha1.OOBStateInProgress, metav1.Condition{
Type: metalv1alpha1.OOBConditionTypeReady,
Status: metav1.ConditionFalse,
Reason: metalv1alpha1.OOBConditionReasonInProgress,
Expand Down Expand Up @@ -640,7 +639,7 @@ func (r *OOBReconciler) processCredentials(ctx context.Context, oob *metalv1alph
apply = apply.WithSpec(util.Ensure(apply.Spec).
WithSecretRef(*oob.Spec.SecretRef))

return r.setCondition(ctx, oob, apply, nil, metalv1alpha1.OOBStateUnready, metav1.Condition{
return r.setCondition(ctx, oob, apply, nil, metalv1alpha1.OOBStateInProgress, metav1.Condition{
Type: metalv1alpha1.OOBConditionTypeReady,
Status: metav1.ConditionFalse,
Reason: metalv1alpha1.OOBConditionReasonInProgress,
Expand All @@ -651,11 +650,7 @@ func (r *OOBReconciler) processCredentials(ctx context.Context, oob *metalv1alph
if oob.Spec.Protocol == nil || (creds.Username == "" && creds.Password == "") {
a, ok := r.macDB.Get(oob.Spec.MACAddress)
if !ok {
return r.setCondition(ctx, oob, apply, nil, metalv1alpha1.OOBStateUnknown, metav1.Condition{
Type: metalv1alpha1.OOBConditionTypeReady,
Status: metav1.ConditionFalse,
Reason: metalv1alpha1.OOBConditionReasonUnknown,
})
return r.setError(ctx, oob, apply, nil, OOBErrorBadCredentials, fmt.Errorf("cannot find MAC address in MAC DB: %s", oob.Spec.MACAddress))
}

if a.Ignore && !metav1.HasAnnotation(oob.ObjectMeta, OOBIgnoreAnnotation) {
Expand Down Expand Up @@ -804,7 +799,7 @@ func (r *OOBReconciler) processCredentials(ctx context.Context, oob *metalv1alph
if oob.Status.State == metalv1alpha1.OOBStateError {
cond, _ := ssa.GetCondition(oob.Status.Conditions, metalv1alpha1.OOBConditionTypeReady)
if strings.HasPrefix(cond.Message, OOBErrorBadCredentials+": ") {
return r.setCondition(ctx, oob, apply, nil, metalv1alpha1.OOBStateUnready, metav1.Condition{
return r.setCondition(ctx, oob, apply, nil, metalv1alpha1.OOBStateInProgress, metav1.Condition{
Type: metalv1alpha1.OOBConditionTypeReady,
Status: metav1.ConditionFalse,
Reason: metalv1alpha1.OOBConditionReasonInProgress,
Expand Down Expand Up @@ -880,18 +875,21 @@ func (r *OOBReconciler) enqueueOOBFromIP(ctx context.Context, obj client.Object)
}

if len(oobList.Items) == 0 && ip.Status.State == ipamv1alpha1.CFinishedIPState && ip.Status.Reserved != nil {
oob := metalv1alpha1.OOB{
ObjectMeta: metav1.ObjectMeta{
Name: mac,
},
}
apply := metalv1alpha1apply.OOB(oob.Name, oob.Namespace).
WithFinalizers(OOBFinalizer).
WithSpec(metalv1alpha1apply.OOBSpec().
WithMACAddress(mac))
err = r.Patch(ctx, &oob, ssa.Apply(apply), client.FieldOwner(OOBFieldManager), client.ForceOwnership)
if err != nil {
log.Error(ctx, fmt.Errorf("cannot apply OOB: %w", err))
_, ok = r.macDB.Get(mac)
if ok {
oob := metalv1alpha1.OOB{
ObjectMeta: metav1.ObjectMeta{
Name: mac,
},
}
apply := metalv1alpha1apply.OOB(oob.Name, oob.Namespace).
WithFinalizers(OOBFinalizer).
WithSpec(metalv1alpha1apply.OOBSpec().
WithMACAddress(mac))
err = r.Patch(ctx, &oob, ssa.Apply(apply), client.FieldOwner(OOBFieldManager), client.ForceOwnership)
if err != nil {
log.Error(ctx, fmt.Errorf("cannot apply OOB: %w", err))
}
}
}

Expand Down
15 changes: 4 additions & 11 deletions internal/controller/oob_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ var _ = Describe("OOB Controller", func() {
By("Expecting the OOB to have no endpoint")
Eventually(Object(oob)).Should(SatisfyAll(
HaveField("Spec.EndpointRef", BeNil()),
HaveField("Status.State", metalv1alpha1.OOBStateUnready),
HaveField("Status.State", metalv1alpha1.OOBStateNoEndpoint),
WithTransform(readyReason, Equal(metalv1alpha1.OOBConditionReasonNoEndpoint)),
))

Expand Down Expand Up @@ -368,7 +368,7 @@ var _ = Describe("OOB Controller", func() {
))
})

It("should set the OOB to unknown if the MAC is unknown", func(ctx SpecContext) {
It("should not create an oob if the MAC is unknown", func(ctx SpecContext) {
By("Creating an IP")
ip := &ipamv1alpha1.IP{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -399,16 +399,9 @@ var _ = Describe("OOB Controller", func() {
Name: ip.Labels[OOBIPMacLabel],
},
}
DeferCleanup(func(ctx SpecContext) {
Expect(k8sClient.Delete(ctx, oob)).To(Succeed())
Eventually(Get(oob)).Should(Satisfy(errors.IsNotFound))
})

By("Expecting OOB to be unknown")
Eventually(Object(oob)).Should(SatisfyAll(
HaveField("Status.State", metalv1alpha1.OOBStateUnknown),
WithTransform(readyReason, Equal(metalv1alpha1.OOBConditionReasonUnknown)),
))
By("Expecting OOB not to exist")
Eventually(Get(oob)).Should(Satisfy(errors.IsNotFound))
})

It("should handle a bad credentials secret", func(ctx SpecContext) {
Expand Down

0 comments on commit ffa72a1

Please sign in to comment.