Repository: metal3-io/baremetal-operator
Pull request #1246 (2023-04-05 05:43:02)

🌱 Disable scheduled workflows from running in forks
Workflows use resources that are only available when run in the [metal3-io/baremetal-operator](https://github.com/metal3-io/baremetal-operator) repository, so workflow run would always fail when they run in a forked repository. They are of no value to fork repositories. They are triggered by the schedule event, so they cause regular annoying and confusing workflow failure notifications for every fork owner. Rather checking these workflows only in upstream repo is enough.



Patch:
diff --git a/.github/workflows/kubesec.yml b/.github/workflows/kubesec.yml
index a481bbd0..b280e7c2 100644
--- a/.github/workflows/kubesec.yml
+++ b/.github/workflows/kubesec.yml
@@ -10,6 +10,9 @@ permissions:
 
 jobs:
   setup:
+    # This workflow is only of value to the metal3-io/baremetal-operator repository and
+    # would always fail in forks
+    if: github.repository == 'metal3-io/baremetal-operator'
     runs-on: ubuntu-20.04
     permissions:
       actions: read
diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml
index 1be12be6..78a43330 100644
--- a/.github/workflows/release.yaml
+++ b/.github/workflows/release.yaml
@@ -13,6 +13,9 @@ jobs:
   build:
     permissions:
       contents: write
+    # This workflow is only of value to the metal3-io/baremetal-operator repository and
+    # would always fail in forks
+    if: github.repository == 'metal3-io/baremetal-operator'
     runs-on: ubuntu-latest
     steps:
       - name: Export RELEASE_TAG var
diff --git a/apis/metal3.io/v1alpha1/baremetalhost_types.go b/apis/metal3.io/v1alpha1/baremetalhost_types.go
index e3987554..d53586ec 100644
--- a/apis/metal3.io/v1alpha1/baremetalhost_types.go
+++ b/apis/metal3.io/v1alpha1/baremetalhost_types.go
@@ -660,18 +660,6 @@ type RebootAnnotationArguments struct {
 	Mode RebootMode `json:"mode"`
 }
 
-type DetachedDeleteAction string
-
-const (
-	DetachedDeleteActionDelay  = "delay"
-	DetachedDeleteActionDelete = "delete"
-)
-
-type DetachedAnnotationArguments struct {
-	// DeleteAction indicates the desired delete logic when the detached annotation is present
-	DeleteAction DetachedDeleteAction `json:"deleteAction,omitempty"`
-}
-
 // Match compares the saved status information with the name and
 // content of a secret object.
 func (cs CredentialsStatus) Match(secret corev1.Secret) bool {
diff --git a/apis/metal3.io/v1alpha1/zz_generated.deepcopy.go b/apis/metal3.io/v1alpha1/zz_generated.deepcopy.go
index 0dcd05ba..031d6294 100644
--- a/apis/metal3.io/v1alpha1/zz_generated.deepcopy.go
+++ b/apis/metal3.io/v1alpha1/zz_generated.deepcopy.go
@@ -382,21 +382,6 @@ func (in DesiredSettingsMap) DeepCopy() DesiredSettingsMap {
 	return *out
 }
 
-// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
-func (in *DetachedAnnotationArguments) DeepCopyInto(out *DetachedAnnotationArguments) {
-	*out = *in
-}
-
-// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DetachedAnnotationArguments.
-func (in *DetachedAnnotationArguments) DeepCopy() *DetachedAnnotationArguments {
-	if in == nil {
-		return nil
-	}
-	out := new(DetachedAnnotationArguments)
-	in.DeepCopyInto(out)
-	return out
-}
-
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *Firmware) DeepCopyInto(out *Firmware) {
 	*out = *in
diff --git a/cmd/make-bm-worker/templates/templates.go b/cmd/make-bm-worker/templates/templates.go
index cb3c8a62..c4bc6832 100644
--- a/cmd/make-bm-worker/templates/templates.go
+++ b/cmd/make-bm-worker/templates/templates.go
@@ -3,8 +3,7 @@ package templates
 import (
 	"bytes"
 	"encoding/base64"
-
-	"github.com/google/safetext/yamltemplate"
+	"text/template"
 )
 
 var templateBody = `---
@@ -103,7 +102,7 @@ func encodeToSecret(input string) string {
 // was a problem rendering it.
 func (t Template) Render() (string, error) {
 	buf := new(bytes.Buffer)
-	tmpl := yamltemplate.Must(yamltemplate.New("yaml_out").Parse(templateBody))
+	tmpl := template.Must(template.New("yaml_out").Parse(templateBody))
 	err := tmpl.Execute(buf, t)
 	return buf.String(), err
 }
diff --git a/cmd/make-virt-host/main.go b/cmd/make-virt-host/main.go
index b1fc2baa..14ed576f 100644
--- a/cmd/make-virt-host/main.go
+++ b/cmd/make-virt-host/main.go
@@ -9,8 +9,7 @@ import (
 	"os"
 	"os/exec"
 	"strings"
-
-	"github.com/google/safetext/yamltemplate"
+	"text/template"
 )
 
 const (
@@ -213,7 +212,7 @@ func main() {
 		Consumer:          strings.TrimSpace(*consumer),
 		ConsumerNamespace: strings.TrimSpace(*consumerNamespace),
 	}
-	t := yamltemplate.Must(yamltemplate.New("yaml_out").Parse(templateBody))
+	t := template.Must(template.New("yaml_out").Parse(templateBody))
 	err = t.Execute(os.Stdout, args)
 	if err != nil {
 		fmt.Fprintf(os.Stderr, "ERROR: %s\n", err)
diff --git a/controllers/metal3.io/host_state_machine.go b/controllers/metal3.io/host_state_machine.go
index 518c017e..e97317ba 100644
--- a/controllers/metal3.io/host_state_machine.go
+++ b/controllers/metal3.io/host_state_machine.go
@@ -1,13 +1,11 @@
 package controllers
 
 import (
-	"encoding/json"
 	"fmt"
 
 	metal3v1alpha1 "github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1"
 	"github.com/metal3-io/baremetal-operator/pkg/provisioner"
 
-	"github.com/go-logr/logr"
 	"github.com/pkg/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
@@ -183,7 +181,7 @@ func (hsm *hostStateMachine) ReconcileState(info *reconcileInfo) (actionRes acti
 		return delayedResult
 	}
 
-	if hsm.checkInitiateDelete(info.log) {
+	if hsm.checkInitiateDelete() {
 		info.log.Info("Initiating host deletion")
 		return actionComplete{}
 	}
@@ -215,7 +213,7 @@ func updateBootModeStatus(host *metal3v1alpha1.BareMetalHost) bool {
 	return true
 }
 
-func (hsm *hostStateMachine) checkInitiateDelete(log logr.Logger) bool {
+func (hsm *hostStateMachine) checkInitiateDelete() bool {
 	if hsm.Host.DeletionTimestamp.IsZero() {
 		// Delete not requested
 		return false
@@ -225,12 +223,11 @@ func (hsm *hostStateMachine) checkInitiateDelete(log logr.Logger) bool {
 	default:
 		hsm.NextState = metal3v1alpha1.StateDeleting
 	case metal3v1alpha1.StateProvisioning, metal3v1alpha1.StateProvisioned:
+		if len(hsm.Host.Finalizers) > 1 {
+			// Allow other finalizers to run before changing state
+			return false
+		}
 		if hsm.Host.OperationalStatus() == metal3v1alpha1.OperationalStatusDetached {
-			if delayDeleteForDetachedHost(hsm.Host) {
-				log.Info("Delaying detached host deletion")
-				deleteDelayedForDetached.Inc()
-				return false
-			}
 			hsm.NextState = metal3v1alpha1.StateDeleting
 		} else {
 			hsm.NextState = metal3v1alpha1.StateDeprovisioning
@@ -256,27 +253,6 @@ func hasDetachedAnnotation(host *metal3v1alpha1.BareMetalHost) bool {
 	return false
 }
 
-func delayDeleteForDetachedHost(host *metal3v1alpha1.BareMetalHost) bool {
-	annotations := host.GetAnnotations()
-	args := metal3v1alpha1.DetachedAnnotationArguments{}
-	val, present := annotations[metal3v1alpha1.DetachedAnnotation]
-
-	// if the host is detached, but missing the annotation, also delay delete
-	// to allow for the host to be re-attached
-	if !present && host.OperationalStatus() == metal3v1alpha1.OperationalStatusDetached {
-		return true
-	}
-
-	if present {
-		if err := json.Unmarshal([]byte(val), &args); err != nil {
-			// default behavior if these are missing or not json is to not delay
-			return false
-		}
-	}
-
-	return args.DeleteAction == metal3v1alpha1.DetachedDeleteActionDelay
-}
-
 func (hsm *hostStateMachine) checkDetachedHost(info *reconcileInfo) (result actionResult) {
 	// If the detached annotation is set we remove any host from the
 	// provisioner and take no further action
diff --git a/controllers/metal3.io/metrics.go b/controllers/metal3.io/metrics.go
index 66611d7f..b9d02197 100644
--- a/controllers/metal3.io/metrics.go
+++ b/controllers/metal3.io/metrics.go
@@ -123,11 +123,6 @@ var provisionerNotReady = prometheus.NewCounter(prometheus.CounterOpts{
 	Help: "Number of times a host is not provision ready",
 })
 
-var deleteDelayedForDetached = prometheus.NewCounter(prometheus.CounterOpts{
-	Name: "metal3_delete_delayed_for_detached_total",
-	Help: "Number of times a host delete action was delayed due to the detached annotation",
-})
-
 func init() {
 	metrics.Registry.MustRegister(
 		reconcileCounters,
@@ -154,8 +149,7 @@ func init() {
 		hostRegistrationRequired,
 		hostUnmanaged,
 		deleteWithoutDeprov,
-		provisionerNotReady,
-		deleteDelayedForDetached)
+		provisionerNotReady)
 }
 
 func hostMetricLabels(request ctrl.Request) prometheus.Labels {
diff --git a/go.mod b/go.mod
index b4b1a385..a9c5ec27 100644
--- a/go.mod
+++ b/go.mod
@@ -4,7 +4,6 @@ go 1.19
 
 require (
 	github.com/go-logr/logr v1.2.3
-	github.com/google/safetext v0.0.0-20230106111101-7156a760e523
 	github.com/gophercloud/gophercloud v1.2.0
 	github.com/metal3-io/baremetal-operator/apis v0.1.2
 	github.com/metal3-io/baremetal-operator/pkg/hardwareutils v0.1.2
diff --git a/go.sum b/go.sum
index c41ba22c..568fab5c 100644
--- a/go.sum
+++ b/go.sum
@@ -205,8 +205,6 @@ github.com/google/pprof v0.0.0-20210601050228-01bbb1931b22/go.mod h1:kpwsk12EmLe
 github.com/google/pprof v0.0.0-20210609004039-a478d1d731e9/go.mod h1:kpwsk12EmLew5upagYY7GY0pfYCcupk39gWOCRROcvE=
 github.com/google/pprof v0.0.0-20210720184732-4bb14d4b1be1/go.mod h1:kpwsk12EmLew5upagYY7GY0pfYCcupk39gWOCRROcvE=
 github.com/google/renameio v0.1.0/go.mod h1:KWCgfxg9yswjAJkECMjeO8J8rahYeXnNhOm40UhjYkI=
-github.com/google/safetext v0.0.0-20230106111101-7156a760e523 h1:i4NsbmB9pD5+Ggp5GZKyvYY6MkjvPE8CIMlkvXFF8gA=
-github.com/google/safetext v0.0.0-20230106111101-7156a760e523/go.mod h1:mJNEy0r5YPHC7ChQffpOszlGB4L1iqjXWpIEKcFpr9s=
 github.com/google/uuid v1.1.2/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
 github.com/google/uuid v1.3.0 h1:t6JiXgmwXMjEs8VusXIJk2BXHsn+wx8BZdTaoZ5fu7I=
 github.com/google/uuid v1.3.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
@@ -738,8 +736,8 @@ gopkg.in/alecthomas/kingpin.v2 v2.2.6/go.mod h1:FMv+mEhP44yOT+4EoQTLFTRgOQ1FBLks
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
+gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f h1:BLraFXnmrev5lT+xlilqcH8XK9/i0At2xKjWk4p6zsU=
 gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
-gopkg.in/check.v1 v1.0.0-20200902074654-038fdea0a05b h1:QRR6H1YWRnHb4Y/HeNFCTJLFVxaq6wH4YuVdsUOr75U=
 gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI=
 gopkg.in/inf.v0 v0.9.1 h1:73M5CoZyi3ZLMOyDlQh031Cx6N9NDJ2Vvfl76EDAgDc=
 gopkg.in/inf.v0 v0.9.1/go.mod h1:cWUDdTG/fYaXco+Dcufb5Vnc6Gp2YChqWtbxRZE0mXw=


Test patch:
diff --git a/controllers/metal3.io/host_state_machine_test.go b/controllers/metal3.io/host_state_machine_test.go
index 9a700a21..1038bd9c 100644
--- a/controllers/metal3.io/host_state_machine_test.go
+++ b/controllers/metal3.io/host_state_machine_test.go
@@ -1075,59 +1075,43 @@ func TestErrorClean(t *testing.T) {
 	}
 }
 
-func TestDeleteWaitsForDetach(t *testing.T) {
+func TestDeleteWaitsForFinalizers(t *testing.T) {
 	tests := []struct {
-		Scenario                  string
-		Host                      *metal3v1alpha1.BareMetalHost
-		ExpectedState             metal3v1alpha1.ProvisioningState
-		ExpectedOperationalStatus metal3v1alpha1.OperationalStatus
+		Scenario      string
+		Host          *metal3v1alpha1.BareMetalHost
+		ExpectedState metal3v1alpha1.ProvisioningState
 	}{
 		{
-			Scenario: "detached-delay",
-			Host: host(metal3v1alpha1.StateProvisioned).
-				SetOperationalStatus(metal3v1alpha1.OperationalStatusDetached).
+			Scenario: "provisioning-2-finalizers",
+			Host: host(metal3v1alpha1.StateProvisioning).
 				setDeletion().
-				setDetached("{\"deleteAction\": \"delay\"}").
+				setFinalizers([]string{metal3v1alpha1.BareMetalHostFinalizer, "other-finalizer"}).
 				build(),
-			ExpectedState:             metal3v1alpha1.StateProvisioned,
-			ExpectedOperationalStatus: metal3v1alpha1.OperationalStatusDetached,
+			ExpectedState: metal3v1alpha1.StateProvisioned,
 		},
 		{
-			Scenario: "detached-delete",
+			Scenario: "provisioned-2-finalizers",
 			Host: host(metal3v1alpha1.StateProvisioned).
-				SetOperationalStatus(metal3v1alpha1.OperationalStatusDetached).
 				setDeletion().
-				setDetached("{\"deleteAction\": \"delete\"}").
+				setFinalizers([]string{metal3v1alpha1.BareMetalHostFinalizer, "other-finalizer"}).
 				build(),
-			ExpectedState:             metal3v1alpha1.StateDeleting,
-			ExpectedOperationalStatus: metal3v1alpha1.OperationalStatusDetached,
+			ExpectedState: metal3v1alpha1.StateProvisioned,
 		},
 		{
-			Scenario: "detached-not-json",
-			Host: host(metal3v1alpha1.StateProvisioned).
-				SetOperationalStatus(metal3v1alpha1.OperationalStatusDetached).
+			Scenario: "provisioning-1-finalizer",
+			Host: host(metal3v1alpha1.StateProvisioning).
 				setDeletion().
-				setDetached("true").
+				setFinalizers([]string{metal3v1alpha1.BareMetalHostFinalizer}).
 				build(),
-			ExpectedState:             metal3v1alpha1.StateDeleting,
-			ExpectedOperationalStatus: metal3v1alpha1.OperationalStatusDetached,
+			ExpectedState: metal3v1alpha1.StateDeprovisioning,
 		},
 		{
-			Scenario: "detached-no-annotation",
+			Scenario: "provisioned-1-finalizer",
 			Host: host(metal3v1alpha1.StateProvisioned).
-				SetOperationalStatus(metal3v1alpha1.OperationalStatusDetached).
 				setDeletion().
+				setFinalizers([]string{metal3v1alpha1.BareMetalHostFinalizer}).
 				build(),
-			ExpectedState:             metal3v1alpha1.StateProvisioned,
-			ExpectedOperationalStatus: metal3v1alpha1.OperationalStatusOK,
-		},
-		{
-			Scenario: "attached",
-			Host: host(metal3v1alpha1.StateProvisioned).
-				setDeletion().
-				build(),
-			ExpectedState:             metal3v1alpha1.StateDeprovisioning,
-			ExpectedOperationalStatus: metal3v1alpha1.OperationalStatusOK,
+			ExpectedState: metal3v1alpha1.StateDeprovisioning,
 		},
 	}
 	for _, tt := range tests {
@@ -1141,7 +1125,6 @@ func TestDeleteWaitsForDetach(t *testing.T) {
 			hsm.ReconcileState(info)
 
 			assert.Equal(t, tt.ExpectedState, tt.Host.Status.Provisioning.State)
-			assert.Equal(t, tt.ExpectedOperationalStatus, tt.Host.Status.OperationalStatus)
 		})
 	}
 }
@@ -1265,11 +1248,8 @@ func (hb *hostBuilder) setDeletion() *hostBuilder {
 	return hb
 }
 
-func (hb *hostBuilder) setDetached(val string) *hostBuilder {
-	if hb.Annotations == nil {
-		hb.Annotations = make(map[string]string, 1)
-	}
-	hb.Annotations[metal3v1alpha1.DetachedAnnotation] = val
+func (hb *hostBuilder) setFinalizers(finalizers []string) *hostBuilder {
+	hb.Finalizers = finalizers
 	return hb
 }