Fix rare case of broken status

ironcore-dev · May 14, 2024 · 97d8f5d · 97d8f5d
1 parent 53a17dd
commit 97d8f5d
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 11 deletions.
diff --git a/Makefile b/Makefile
@@ -50,7 +50,7 @@ vet: ## Run go vet against code.
 
 .PHONY: test
 test: ## Run tests.
-	@go run github.com/onsi/ginkgo/v2/ginkgo -r --race --randomize-suites --keep-going --randomize-all --repeat=1
+	@go run github.com/onsi/ginkgo/v2/ginkgo -r --race --randomize-suites --keep-going --randomize-all --repeat=300 -vv
 
 .PHONY: lint
 lint: ## Run golangci-lint linter & yamllint.

diff --git a/internal/controller/oob_controller.go b/internal/controller/oob_controller.go
@@ -128,7 +128,7 @@ type access struct {
 	Protocol           metalv1alpha1.Protocol `yaml:"protocol"`
 	Flags              map[string]string      `yaml:"flags"`
 	DefaultCredentials []bmc.Credentials      `yaml:"defaultCredentials"`
-	Type               bmc.Typ                `yaml:"type"`
+	Type               metalv1alpha1.OOBType  `yaml:"type"`
 }
 
 type ctxkOOBHost struct{}
@@ -147,6 +147,7 @@ func (r *OOBReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.R
 	if err != nil {
 		return ctrl.Result{}, client.IgnoreNotFound(fmt.Errorf("cannot get OOB: %w", err))
 	}
+	log.Info(ctx, "OOB", "object", oob) //fixme
 
 	if !oob.DeletionTimestamp.IsZero() {
 		return ctrl.Result{}, r.finalize(ctx, &oob)
@@ -388,10 +389,13 @@ func (r *OOBReconciler) runPhase(ctx context.Context, oob *metalv1alpha1.OOB, ph
 		apply = metalv1alpha1apply.OOB(oob.Name, oob.Namespace).WithStatus(status)
 
 		log.Debug(ctx, "Applying status")
+		log.Info(ctx, "STATUS BEFOR", "object", oob)  //fixme
+		log.Info(ctx, "STATUS APPLY", "apply", apply) //fixme
 		err = r.Status().Patch(ctx, oob, ssa.Apply(apply), client.FieldOwner(OOBFieldManager), client.ForceOwnership)
 		if err != nil {
 			return ctx, false, fmt.Errorf("cannot apply OOB status: %w", err)
 		}
+		log.Info(ctx, "STATUS AFTER", "object", oob) //fixme
 	}
 
 	cond, ok := ssa.GetCondition(oob.Status.Conditions, metalv1alpha1.OOBConditionTypeReady)
@@ -709,7 +713,7 @@ func (r *OOBReconciler) processCredentials(ctx context.Context, oob *metalv1alph
 		}
 	}
 
-	if oob.Spec.Protocol == nil || (creds.Username == "" && creds.Password == "") {
+	if oob.Spec.Protocol == nil || oob.Status.Type == "" || (creds.Username == "" && creds.Password == "") {
 		a, ok := r.macDB.Get(oob.Spec.MACAddress)
 		if !ok {
 			return r.setError(ctx, oob, apply, status, OOBErrorBadCredentials, fmt.Errorf("cannot find MAC address in MAC DB: %s", oob.Spec.MACAddress))
@@ -734,7 +738,7 @@ func (r *OOBReconciler) processCredentials(ctx context.Context, oob *metalv1alph
 		oob.Spec.Protocol = &a.Protocol
 		oob.Spec.Flags = a.Flags
 		defaultCreds = a.DefaultCredentials
-		oob.Status.Type = metalv1alpha1.OOBType(a.Type)
+		oob.Status.Type = a.Type
 		log.Debug(ctx, "Setting protocol, flags, and type")
 		if apply == nil {
 			var err error
@@ -748,14 +752,15 @@ func (r *OOBReconciler) processCredentials(ctx context.Context, oob *metalv1alph
 				WithName(oob.Spec.Protocol.Name).
 				WithPort(oob.Spec.Protocol.Port)).
 			WithFlags(oob.Spec.Flags))
-		if a.Type != "" {
-			applyst, err := metalv1alpha1apply.ExtractOOBStatus(oob, OOBFieldManager)
-			if err != nil {
-				return ctx, nil, nil, fmt.Errorf("cannot extract OOB status: %w", err)
-			}
-			status = util.Ensure(applyst.Status).
-				WithType(metalv1alpha1.OOBType(a.Type))
+		log.Info(ctx, "APPLY OOB", "obj", oob)
+		applyst, err := metalv1alpha1apply.ExtractOOBStatus(oob, OOBFieldManager)
+		if err != nil {
+			return ctx, nil, nil, fmt.Errorf("cannot extract OOB status: %w", err)
 		}
+		log.Info(ctx, "APPLY EXT", "st", applyst.Status) //fixme
+		status = util.Ensure(applyst.Status).
+			WithType(a.Type)
+		log.Info(ctx, "APPLY ENS", "st", status) //fixme
 	}
 
 	b, err := bmc.NewBMC(string(oob.Spec.Protocol.Name), oob.Spec.Flags, host, oob.Spec.Protocol.Port, creds, expiration)
@@ -1236,6 +1241,21 @@ func loadMacDB(dbFile string) (util.PrefixMap[access], error) {
 
 	db := make(util.PrefixMap[access], len(dbf.MACs))
 	for _, m := range dbf.MACs {
+		if m.access.Protocol.Name == "" {
+			return nil, fmt.Errorf("prefix %s has no protocol name", m.Prefix)
+		}
+		if len(m.access.DefaultCredentials) == 0 {
+			return nil, fmt.Errorf("prefix %s has no default credentials", m.Prefix)
+		}
+		for _, dc := range m.access.DefaultCredentials {
+			if dc.Username == "" && dc.Password == "" {
+				return nil, fmt.Errorf("prefix %s has invalid default credentials", m.Prefix)
+			}
+		}
+		if m.access.Type == "" {
+			return nil, fmt.Errorf("prefix %s has no type", m.Prefix)
+		}
+
 		db[m.Prefix] = m.access
 	}