Handling propagation of Detached Annotation

When a HostClaim has a detached annotation, we propagate it to the
BareMetalHost iff there is a HostDeployPolicy compatible with the claim
that allows such a propagation.

We add a manager flag in the "arguments" of the annotation. HostClaim
do not set it to true and if the annotation does not appear in the
HostClaim, it is not removed from the BareMetalHost if the manager
field is set to true.

This way the administrator can still force detachment.

Signed-off-by: Pierre Crégut <pierre.cregut@orange.com>

Author: pierrecregut

apis/metal3.io/v1alpha1/baremetalhost_types.go

@@ -699,6 +699,10 @@ const (
 type DetachedAnnotationArguments struct {
 	// DeleteAction indicates the desired delete logic when the detached annotation is present
 	DeleteAction DetachedDeleteAction `json:"deleteAction,omitempty"`
+	// Manager is an informal flag that tells that the annotation was set by an administrator.
+	// HostClaim controller will always set it to false and will never delete an annotation with
+	// manager set to true.
+	Manager bool `json:"manager,omitempty"`
 }
 
 // Match compares the saved status information with the name and

apis/metal3.io/v1alpha1/hostdeploypolicy_types.go

@@ -25,6 +25,9 @@ type HostDeployPolicySpec struct {
 	// HostClaimNamespaces constrains the namespaces of the HostClaims allowed
 	// to bind the BareMetalHosts in the same namespace as the HostDeployPolicy
 	HostClaimNamespaces *HostClaimNamespaces `json:"hostClaimNamespaces,omitempty"`
+	// AllowsDetachedMode is a boolean flag specifying if the hostClaim can set the
+	// detached annotation of the BareMetalHost.
+	AllowsDetachedMode bool `json:"allowsDetachedMode,omitempty"`
 }
 
 type HostClaimNamespaces struct {

internal/testutil/common.go

@@ -360,3 +360,8 @@ func (hb *HostDeployPolicyBuilder) AcceptRegexp(re string) *HostDeployPolicyBuil
 	spec.HostClaimNamespaces.NameMatches = re
 	return hb
 }
+
+func (hb *HostDeployPolicyBuilder) AllowsDetachedMode() *HostDeployPolicyBuilder {
+	hb.hostDeployPolicy.Spec.AllowsDetachedMode = true
+	return hb
+}

pkg/hostclaim/hostclaim_manager.go

@@ -19,6 +19,7 @@ package hostclaim
 import (
 	"context"
 	"crypto/rand"
+	"encoding/json"
 	"errors"
 	"math/big"
 	"regexp"
@@ -200,6 +201,11 @@ func (m *HostManager) Update(ctx context.Context) error {
 	if syncReboot(m.HostClaim.Annotations, bmh.Annotations) {
 		updated = true
 	}
+	if upd, err := m.syncDetached(ctx, bmh.Namespace, m.HostClaim.Annotations, bmh.Annotations); err != nil {
+		return err
+	} else if upd {
+		updated = true
+	}
 	if updated {
 		m.Log.Info("Update the BareMetalHost spec: changes detected.")
 		err = m.client.Update(ctx, bmh)
@@ -535,56 +541,59 @@ func (m *HostManager) acceptableNamespaces(ctx context.Context, namespace string
 		nsLabels = map[string]string{}
 	}
 	namespaces := NewSet[string]()
-LOOP_POLICY:
 	for _, hostDeployPolicy := range hostdeploypolicies.Items {
-		log := m.Log.WithValues("policyNamespace", hostDeployPolicy.Namespace, "policyName", hostDeployPolicy.Name)
-		if SetContains(namespaces, hostDeployPolicy.Namespace) {
-			// namespace already added, no reason to check this hdp.
-			continue
-		}
-		constraints := hostDeployPolicy.Spec.HostClaimNamespaces
-		if constraints == nil {
-			log.V(1).Info("Ignoring HostDeployPolicy without constraint")
-			continue
+		if accept, err := m.checkPolicy(&hostDeployPolicy, hostNs, nsLabels); err != nil {
+			return nil, err
+		} else if accept {
+			AddSet(namespaces, hostDeployPolicy.Namespace)
 		}
-		if constraints.Names != nil && !slices.Contains(constraints.Names, hostNs) {
-			log.V(1).Info("Ignoring HostDeployPolicy because claim namespace not in names", "names", constraints.Names)
-			continue
+	}
+	m.Log.Info("Acceptable namespaces", "namespaces", namespaces)
+	return namespaces, nil
+}
+
+func (m *HostManager) checkPolicy(hostDeployPolicy *metal3api.HostDeployPolicy, hostNs string, hostNsLabels map[string]string) (bool, error) {
+	log := m.Log.WithValues("policyNamespace", hostDeployPolicy.Namespace, "policyName", hostDeployPolicy.Name)
+	constraints := hostDeployPolicy.Spec.HostClaimNamespaces
+	if constraints == nil {
+		m.Log.V(1).Info("Ignoring HostDeployPolicy without constraint")
+		return false, nil
+	}
+	if constraints.Names != nil && !slices.Contains(constraints.Names, hostNs) {
+		log.V(1).Info("Ignoring HostDeployPolicy because claim namespace not in names", "names", constraints.Names)
+		return false, nil
+	}
+	if constraints.NameMatches != "" {
+		b, err := regexp.MatchString(constraints.NameMatches, hostNs)
+		if err != nil {
+			log.Error(
+				err, "Error during regexp matching on HostClaim namespace (bad regexp)",
+				"regexp", constraints.NameMatches)
+			return false, err
 		}
-		if constraints.NameMatches != "" {
-			b, err := regexp.MatchString(constraints.NameMatches, hostNs)
-			if err != nil {
-				log.Error(
-					err, "Error during regexp matching on HostClaim namespace (bad regexp)",
-					"regexp", constraints.NameMatches)
-				return nil, err
-			}
-			if !b {
-				log.V(1).Info("Ignoring HostDeployPolicy because claim namespace does not match regex")
-				continue
-			}
+		if !b {
+			log.V(1).Info("Ignoring HostDeployPolicy because claim namespace does not match regex")
+			return false, nil
 		}
-		// Behaves as a 'forall' on labels
-		for _, pair := range constraints.HasLabels {
-			if v, ok := nsLabels[pair.Name]; ok {
-				if pair.Value != "" && v != pair.Value {
-					log.V(1).Info(
-						"Ignoring HostDeployPolicy because claim namespace labels does not have correct value",
-						"label", pair.Name, "value", v, "expected", pair.Value)
-					continue LOOP_POLICY
-				}
-			} else {
+	}
+	// Behaves as a 'forall' on labels
+	for _, pair := range constraints.HasLabels {
+		if v, ok := hostNsLabels[pair.Name]; ok {
+			if pair.Value != "" && v != pair.Value {
 				log.V(1).Info(
-					"Ignoring HostDeployPolicy because claim namespace labels does not have a required label",
-					"label", pair.Name)
-				continue LOOP_POLICY
+					"Ignoring HostDeployPolicy because claim namespace labels does not have correct value",
+					"label", pair.Name, "value", v, "expected", pair.Value)
+				return false, nil
 			}
+		} else {
+			log.V(1).Info(
+				"Ignoring HostDeployPolicy because claim namespace labels does not have a required label",
+				"label", pair.Name)
+			return false, nil
 		}
-		log.V(1).Info("Accepting namespace because of HostDeployPolicy", "namespace", hostDeployPolicy.Namespace)
-		AddSet(namespaces, hostDeployPolicy.Namespace)
 	}
-	m.Log.Info("Acceptable namespaces", "namespaces", namespaces)
-	return namespaces, nil
+	log.V(1).Info("Accepting namespace because of HostDeployPolicy", "namespace", hostDeployPolicy.Namespace)
+	return true, nil
 }
 
 func (m *HostManager) hostLabelSelectorForHostClaim() (labels.Selector, error) {
@@ -740,6 +749,42 @@ func syncReboot(hostMap, bmhMap map[string]string) bool {
 	return updated
 }
 
+// synchronize detached annotation from hostMap to bmhMap.
+func (m *HostManager) syncDetached(ctx context.Context, bmhNs string, hostMap, bmhMap map[string]string) (bool, error) {
+	if annotValue, ok := hostMap[metal3api.DetachedAnnotation]; ok {
+		if _, ok := bmhMap[metal3api.DetachedAnnotation]; ok {
+			return false, nil
+		}
+		if compatiblePolicies, err := m.GetCompatiblePolicies(ctx, bmhNs); err != nil {
+			return false, err
+		} else if !slices.ContainsFunc(
+			compatiblePolicies,
+			func(hdp *metal3api.HostDeployPolicy) bool {
+				return hdp.Spec.AllowsDetachedMode
+			},
+		) {
+			m.Log.Info("HostClaim not allowed to detach underlying BareMetalHost")
+			return false, err
+		}
+		inputArgs := metal3api.DetachedAnnotationArguments{}
+		_ = json.Unmarshal([]byte(annotValue), &inputArgs)
+		args := metal3api.DetachedAnnotationArguments{DeleteAction: inputArgs.DeleteAction}
+		if annot, err := json.Marshal(args); err != nil {
+			return false, err
+		} else {
+			bmhMap[metal3api.DetachedAnnotation] = string(annot)
+		}
+		return true, nil
+	} else if annotValue, ok := bmhMap[metal3api.DetachedAnnotation]; ok {
+		args := metal3api.DetachedAnnotationArguments{}
+		if err := json.Unmarshal([]byte(annotValue), &args); err != nil || args.Manager {
+			return false, nil
+		}
+		delete(bmhMap, metal3api.DetachedAnnotation)
+	}
+	return false, nil
+}
+
 type Set[T comparable] = map[T]struct{}
 
 func NewSet[T comparable]() Set[T] {
@@ -754,3 +799,33 @@ func SetContains[T comparable](m Set[T], s T) bool {
 	_, ok := m[s]
 	return ok
 }
+
+// GetCompatiblePolicies find all the HostDeployPolicies in the namespace of the bareMetalHost that
+// accept the namespace of the current HostClaim.
+func (m *HostManager) GetCompatiblePolicies(ctx context.Context, bmhNs string) ([]*metal3api.HostDeployPolicy, error) {
+	policiesInNs := &metal3api.HostDeployPolicyList{}
+	if err := m.client.List(ctx, policiesInNs, client.InNamespace(bmhNs)); err != nil {
+		m.Log.Error(err, "Cannot access HostDeployPolicies", "bmhNamespace", bmhNs)
+		return nil, err
+	}
+	hostNs := m.HostClaim.Namespace
+	hostNsResource := &corev1.Namespace{}
+	if err := m.client.Get(ctx, client.ObjectKey{Name: hostNs}, hostNsResource); err != nil {
+		m.Log.Error(err, "cannot access the namespace of the claim")
+		return nil, err
+	}
+	nsLabels := hostNsResource.Labels
+	if nsLabels == nil {
+		nsLabels = map[string]string{}
+	}
+
+	var compatiblePolicies []*metal3api.HostDeployPolicy
+	for _, hdp := range policiesInNs.Items {
+		if accept, err := m.checkPolicy(&hdp, hostNs, nsLabels); err != nil {
+			return nil, err
+		} else if accept {
+			compatiblePolicies = append(compatiblePolicies, &hdp)
+		}
+	}
+	return compatiblePolicies, nil
+}

pkg/hostclaim/hostclaim_manager_test.go

@@ -564,6 +564,49 @@ var _ = Describe("HostClaim manager", func() {
 		},
 	)
 
+	type testCaseSyncDetach struct {
+		AcceptHostClaimNs string
+		InHostClaimAnnot  string
+		InBmhAnnot        string
+		OutHostClaimAnnot string
+		OutBmhAnnot       string
+	}
+
+	DescribeTable("test syncDetach",
+		func(tc testCaseSyncDetach) {
+			ctx := context.TODO()
+			ns := NewNamespace(HostclaimNamespace).Build()
+
+			annotHc := map[string]string{}
+			annotBmh := map[string]string{}
+			if tc.InHostClaimAnnot != "" {
+				annotHc = map[string]string{metal3api.DetachedAnnotation: tc.InHostClaimAnnot}
+			}
+			if tc.InBmhAnnot != "" {
+				annotBmh = map[string]string{metal3api.DetachedAnnotation: tc.InBmhAnnot}
+			}
+
+			bmh := NewBaremetalhost("bmh", "ns", metal3api.StateAvailable).SetAnnotations(annotBmh).SetConsumerRef(defaultConsumerRef).Build()
+			hostClaim := NewHostclaim(HostclaimName).SetAssociatedBMH("ns", "bmh").SetAnnotations(annotHc).Build()
+			hdp := NewHostdeploypolicy("hdp", "ns").AcceptNames([]string{tc.AcceptHostClaimNs}).AllowsDetachedMode().Build()
+			fakeClient := fake.NewClientBuilder().WithScheme(setupScheme()).WithObjects(bmh, hostClaim, ns, hdp).Build()
+			hostMgr, err := NewHostManager(fakeClient, GinkgoLogr, hostClaim, fakeClient)
+			Expect(err).NotTo(HaveOccurred())
+			hostMgr.syncDetached(ctx, "ns", hostClaim.Annotations, bmh.Annotations)
+			outBmhAnnot := ""
+			if bmh.Annotations != nil {
+				outBmhAnnot = bmh.Annotations[metal3api.DetachedAnnotation]
+			}
+			Expect(outBmhAnnot).To(Equal(tc.OutBmhAnnot))
+		},
+		Entry("not allowed", testCaseSyncDetach{AcceptHostClaimNs: "other", InHostClaimAnnot: "{}", OutBmhAnnot: ""}),
+		Entry("allowed", testCaseSyncDetach{AcceptHostClaimNs: HostclaimNamespace, InHostClaimAnnot: "{}", OutBmhAnnot: "{}"}),
+		Entry("sanitize", testCaseSyncDetach{AcceptHostClaimNs: HostclaimNamespace, InHostClaimAnnot: "{\"deleteAction\": \"delay\", \"manager\": true}", OutBmhAnnot: "{\"deleteAction\":\"delay\"}"}),
+		Entry("no override", testCaseSyncDetach{AcceptHostClaimNs: HostclaimNamespace, InHostClaimAnnot: "{}", InBmhAnnot: "zzz", OutBmhAnnot: "zzz"}),
+		Entry("delete", testCaseSyncDetach{AcceptHostClaimNs: HostclaimNamespace, InHostClaimAnnot: "", InBmhAnnot: "{}", OutBmhAnnot: ""}),
+		Entry("no delete", testCaseSyncDetach{AcceptHostClaimNs: HostclaimNamespace, InHostClaimAnnot: "", InBmhAnnot: "{\"manager\": true}", OutBmhAnnot: "{\"manager\": true}"}),
+	)
+
 	type testCaseUpdate struct {
 		HostClaim  *metal3api.HostClaim
 		ExpectFail bool