Merge pull request #1000 from jacobweinstock/builtin-workflow-features

Workflow boot features, print columns, and Workflow Conditions:

## Description

<!--- Please describe what this PR is going to change -->
1. Adds fields in the Workflow spec that trigger handling of setting hardware `allowPxe` field and getting machines into a netbooting state.

    **`toggleAllowNetboot`:** indicates whether the controller should toggle the field in the associated hardware for allowing PXE booting. This will be enabled before a Workflow is executed and disabled after the Workflow has completed successfully. A HardwareRef must be provided.

    **`oneTimeNetboot`:** indicates whether the controller should create a `job.bmc.tinkerbell.org` object for getting the associated hardware into a netbooting state. A HardwareRef that contains a spec.bmcRef must be provided.

    Current Workflow spec example:
 
   ```yaml
    apiVersion: tinkerbell.org/v1alpha1
    kind: Workflow
    metadata:
      name: example1
      namespace: tink-system
    spec:
      hardwareMap:
        device_1: 01:02:03:04:05:06
      templateRef: myTemplate
      hardwareRef: example1
    ```

    New Workflow spec example:
 
   ```yaml
    apiVersion: tinkerbell.org/v1alpha1
    kind: Workflow
    metadata:
      name: example1
      namespace: tink-system
    spec:
      hardwareMap:
        device_1: 01:02:03:04:05:06
      templateRef: myTemplate
      hardwareRef: example1
      bootOptions:
        toggleAllowNetboot: true
        oneTimeNetboot: true
    ```

2. Adds the following columns to `kubectl get workflow`: `CURRENT-ACTION` and `TEMPLATE-RENDERING`.

    ```bash
    ❯ kubectl get wf -n tink-system 
    NAME         TEMPLATE   STATE           CURRENT-ACTION   TEMPLATE-RENDERING
    testing      testing    STATE_SUCCESS   action 7         successful
    ```

3. Introduces [Conditions](https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#typical-status-properties) to the Workflow spec. The following are the initial conditions.

    ```
    NetbootJobFailed
    NetbootJobComplete
    NetbootJobRunning
    NetbootJobSetupFailed
    NetbootJobSetupComplete
    ToggleAllowNetbootTrue
    ToggleAllowNetbootFalse
    TemplateRenderedSuccess
    ```

## Why is this needed

<!--- Link to issue you have raised -->

Fixes: #

## How Has This Been Tested?
<!--- Please describe in detail how you tested your changes. -->
<!--- Include details of your testing environment, and the tests you ran to -->
<!--- see how your change affects other areas of the code, etc. -->


## How are existing users impacted? What migration steps/scripts do we need?

<!--- Fixes a bug, unblocks installation, removes a component of the stack etc -->
<!--- Requires a DB migration script, etc. -->


## Checklist:

I have:

- [ ] updated the documentation and/or roadmap (if required)
- [ ] added unit or e2e tests
- [ ] provided instructions on how to upgrade

.golangci.yml

@@ -80,10 +80,11 @@ linters-settings:
       - name: var-naming
       - name: unconditional-recursion
       - name: waitgroup-by-value
-  staticcheck:
-    go: "1.18"
-  unused:
-    go: "1.18"
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#struct-tag
+      - name: struct-tag
+        arguments:
+          - "json,inline"
+          - "protobuf,casttype"
 output:
   sort-results: true
 linters:
@@ -99,7 +100,7 @@ linters:
     - errname
     - errorlint
     - exhaustive
-    - exportloopref
+    - copyloopvar
     - forcetypeassert
     - gocognit
     - goconst

Makefile

@@ -120,12 +120,12 @@ generate-crds: $(CONTROLLER_GEN) $(YAMLFMT)
 	$(YAMLFMT) ./config/crd/bases/* ./config/webhook/*
 
 .PHONY: generate-rbac
-generate-rbac: $(CONTROLLER_GEN) $(YAMLFMT)
+generate-rbac: generate-controller-rbac generate-server-rbac $(CONTROLLER_GEN) $(YAMLFMT)
 
 .PHONY: generate-controller-rbac
-generate-manager-rbac:
+generate-controller-rbac:
 	$(CONTROLLER_GEN) \
-		paths=./internal/workflow/... \
+		paths=./internal/deprecated/workflow/... \
 		output:rbac:dir=./config/manager-rbac/ \
 		rbac:roleName=manager-role
 	$(YAMLFMT) ./config/rbac/*
@@ -151,14 +151,14 @@ out/release/default/kustomization.yaml: config/default/kustomization.yaml
 	mkdir -p out/
 	cp -a config/ out/release/
 
-out/release/tink.yaml: generate-manifests out/release/default/kustomization.yaml $(KUSTOMIZE)
+out/release/tink.yaml: generate-manifests out/release/default/kustomization.yaml $(KUSTOMIZE) $(YAMLFMT)
 	(
 		cd out/release/default && \
 		$(KUSTOMIZE) edit set image server=$(TINK_SERVER_IMAGE):$(TINK_CONTROLLER_TAG) controller=$(TINK_CONTROLLER_IMAGE):$(TINK_CONTROLLER_TAG) && \
 		$(KUSTOMIZE) edit set namespace $(NAMESPACE) \
 	)
 	$(KUSTOMIZE) build out/release/default -o $@
-	prettier --write $@
+	$(YAMLFMT) $@
 
 .PHONY: release-manifests
 release-manifests: ## Builds the manifests to publish with a release.

Tools.mk

@@ -23,7 +23,7 @@ PROTOC_GEN_GO_GRPC     := $(TOOLS_DIR)/protoc-gen-go-grpc
 PROTOC_GEN_GO_VER := v1.28
 PROTOC_GEN_GO     := $(TOOLS_DIR)/protoc-gen-go
 
-CONTROLLER_GEN_VER := v0.15
+CONTROLLER_GEN_VER := v0.16.3
 CONTROLLER_GEN     := $(TOOLS_DIR)/controller-gen-$(CONTROLLER_GEN_VER)
 
 KUSTOMIZE_VER := v4.5
@@ -32,7 +32,7 @@ KUSTOMIZE     := $(TOOLS_DIR)/kustomize-$(KUSTOMIZE_VER)
 SETUP_ENVTEST_VER := v0.0.0-20220304125252-9ee63fc65a97
 SETUP_ENVTEST     := $(TOOLS_DIR)/setup-envtest-$(SETUP_ENVTEST_VER)
 
-GOLANGCI_LINT_VER := v1.52
+GOLANGCI_LINT_VER := v1.61
 GOLANGCI_LINT     := $(TOOLS_DIR)/golangci-lint-$(GOLANGCI_LINT_VER)
 
 YAMLFMT_VER := v0.6

api/v1alpha1/workflow_methods.go

@@ -60,7 +60,7 @@ func (w *Workflow) getTaskActionInfo() taskInfo {
 	INNER:
 		for ai, action := range task.Actions {
 			// Find the first non-successful action
-			switch action.Status {
+			switch action.Status { //nolint:exhaustive // WorkflowStateWaiting is only used in Workflows not Actions.
 			case WorkflowStateSuccess:
 				actionIndex++
 				continue

api/v1alpha1/workflow_test.go

@@ -4,6 +4,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/google/go-cmp/cmp"
 	"github.com/tinkerbell/tink/internal/testtime"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
@@ -408,3 +409,44 @@ func TestWorkflowMethods(t *testing.T) {
 		})
 	}
 }
+
+func TestSetCondition(t *testing.T) {
+	tests := map[string]struct {
+		ExistingConditions []WorkflowCondition
+		WantConditions     []WorkflowCondition
+		Condition          WorkflowCondition
+	}{
+		"update existing condition": {
+			ExistingConditions: []WorkflowCondition{
+				{Type: ToggleAllowNetbootTrue, Status: metav1.ConditionTrue},
+				{Type: ToggleAllowNetbootFalse, Status: metav1.ConditionTrue},
+			},
+			WantConditions: []WorkflowCondition{
+				{Type: ToggleAllowNetbootTrue, Status: metav1.ConditionFalse},
+				{Type: ToggleAllowNetbootFalse, Status: metav1.ConditionTrue},
+			},
+			Condition: WorkflowCondition{Type: ToggleAllowNetbootTrue, Status: metav1.ConditionFalse},
+		},
+		"append new condition": {
+			ExistingConditions: []WorkflowCondition{
+				{Type: ToggleAllowNetbootTrue, Status: metav1.ConditionTrue},
+			},
+			WantConditions: []WorkflowCondition{
+				{Type: ToggleAllowNetbootTrue, Status: metav1.ConditionTrue},
+				{Type: ToggleAllowNetbootFalse, Status: metav1.ConditionFalse},
+			},
+			Condition: WorkflowCondition{Type: ToggleAllowNetbootFalse, Status: metav1.ConditionFalse},
+		},
+	}
+	for name, tt := range tests {
+		t.Run(name, func(t *testing.T) {
+			w := &WorkflowStatus{
+				Conditions: tt.ExistingConditions,
+			}
+			w.SetCondition(tt.Condition)
+			if !cmp.Equal(tt.WantConditions, w.Conditions) {
+				t.Errorf("SetCondition() mismatch (-want +got):\n%s", cmp.Diff(tt.WantConditions, w.Conditions))
+			}
+		})
+	}
+}

api/v1alpha1/workflow_types.go

@@ -2,40 +2,166 @@ package v1alpha1
 
 import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
 )
 
-type WorkflowState string
+func init() {
+	SchemeBuilder.Register(&Workflow{}, &WorkflowList{})
+}
+
+type (
+	WorkflowState         string
+	WorkflowConditionType string
+	TemplateRendering     string
+)
 
 const (
+	WorkflowStateWaiting = WorkflowState("STATE_WAITING")
 	WorkflowStatePending = WorkflowState("STATE_PENDING")
 	WorkflowStateRunning = WorkflowState("STATE_RUNNING")
+	WorkflowStateSuccess = WorkflowState("STATE_SUCCESS")
 	WorkflowStateFailed  = WorkflowState("STATE_FAILED")
 	WorkflowStateTimeout = WorkflowState("STATE_TIMEOUT")
-	WorkflowStateSuccess = WorkflowState("STATE_SUCCESS")
+
+	NetbootJobFailed        WorkflowConditionType = "NetbootJobFailed"
+	NetbootJobComplete      WorkflowConditionType = "NetbootJobComplete"
+	NetbootJobRunning       WorkflowConditionType = "NetbootJobRunning"
+	NetbootJobSetupFailed   WorkflowConditionType = "NetbootJobSetupFailed"
+	NetbootJobSetupComplete WorkflowConditionType = "NetbootJobSetupComplete"
+	ToggleAllowNetbootTrue  WorkflowConditionType = "AllowNetbootTrue"
+	ToggleAllowNetbootFalse WorkflowConditionType = "AllowNetbootFalse"
+	TemplateRenderedSuccess WorkflowConditionType = "TemplateRenderedSuccess"
+
+	TemplateRenderingSuccessful TemplateRendering = "successful"
+	TemplateRenderingFailed     TemplateRendering = "failed"
 )
 
+// +kubebuilder:subresource:status
+// +kubebuilder:object:root=true
+// +kubebuilder:resource:path=workflows,scope=Namespaced,categories=tinkerbell,shortName=wf,singular=workflow
+// +kubebuilder:storageversion
+// +kubebuilder:printcolumn:JSONPath=".spec.templateRef",name=Template,type=string
+// +kubebuilder:printcolumn:JSONPath=".status.state",name=State,type=string
+// +kubebuilder:printcolumn:JSONPath=".status.currentAction",name=Current-Action,type=string
+// +kubebuilder:printcolumn:JSONPath=".status.templateRending",name=Template-Rendering,type=string
+
+// Workflow is the Schema for the Workflows API.
+type Workflow struct {
+	metav1.TypeMeta   `json:",inline"`
+	metav1.ObjectMeta `json:"metadata,omitempty"`
+
+	Spec   WorkflowSpec   `json:"spec,omitempty"`
+	Status WorkflowStatus `json:"status,omitempty"`
+}
+
+// +kubebuilder:object:root=true
+
+// WorkflowList contains a list of Workflows.
+type WorkflowList struct {
+	metav1.TypeMeta `json:",inline"`
+	metav1.ListMeta `json:"metadata,omitempty"`
+	Items           []Workflow `json:"items"`
+}
+
 // WorkflowSpec defines the desired state of Workflow.
 type WorkflowSpec struct {
 	// Name of the Template associated with this workflow.
 	TemplateRef string `json:"templateRef,omitempty"`
 
 	// Name of the Hardware associated with this workflow.
+	// +optional
 	HardwareRef string `json:"hardwareRef,omitempty"`
 
-	// A mapping of template devices to hadware mac addresses
+	// A mapping of template devices to hadware mac addresses.
 	HardwareMap map[string]string `json:"hardwareMap,omitempty"`
+
+	// BootOptions are options that control the booting of Hardware.
+	BootOptions BootOptions `json:"bootOptions,omitempty"`
+}
+
+// BootOptions are options that control the booting of Hardware.
+type BootOptions struct {
+	// ToggleAllowNetboot indicates whether the controller should toggle the field in the associated hardware for allowing PXE booting.
+	// This will be enabled before a Workflow is executed and disabled after the Workflow has completed successfully.
+	// A HardwareRef must be provided.
+	// +optional
+	ToggleAllowNetboot bool `json:"toggleAllowNetboot,omitempty"`
+	// OneTimeNetboot indicates whether the controller should create a job.bmc.tinkerbell.org object for getting the associated hardware
+	// into a netbooting state.
+	// A HardwareRef that contains a spec.BmcRef must be provided.
+	// +optional
+	OneTimeNetboot bool `json:"oneTimeNetboot,omitempty"`
+}
+
+// BootOptionsStatus holds the state of any boot options.
+type BootOptionsStatus struct {
+	// OneTimeNetboot holds the state of a specific job.bmc.tinkerbell.org object created.
+	// Only used when BootOptions.OneTimeNetboot is true.
+	OneTimeNetboot OneTimeNetbootStatus `json:"netbootJob,omitempty"`
 }
 
-// WorkflowStatus defines the observed state of Workflow.
+// WorkflowStatus defines the observed state of a Workflow.
 type WorkflowStatus struct {
-	// State is the state of the workflow in Tinkerbell.
+	// State is the current overall state of the Workflow.
 	State WorkflowState `json:"state,omitempty"`
 
-	// GlobalTimeout represents the max execution time
+	// CurrentAction is the action that is currently in the running state.
+	CurrentAction string `json:"currentAction,omitempty"`
+
+	// BootOptions holds the state of any boot options.
+	BootOptions BootOptionsStatus `json:"bootOptions,omitempty"`
+
+	// TemplateRendering indicates whether the template was rendered successfully.
+	// Possible values are "successful" or "failed" or "unknown".
+	TemplateRendering TemplateRendering `json:"templateRending,omitempty"`
+
+	// GlobalTimeout represents the max execution time.
 	GlobalTimeout int64 `json:"globalTimeout,omitempty"`
 
-	// Tasks are the tasks to be completed
+	// Tasks are the tasks to be run by the worker(s).
 	Tasks []Task `json:"tasks,omitempty"`
+
+	// Conditions are the latest available observations of an object's current state.
+	//
+	// +optional
+	// +patchMergeKey=type
+	// +patchStrategy=merge
+	// +listType=atomic
+	Conditions []WorkflowCondition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type" protobuf:"bytes,1,rep,name=conditions"`
+}
+
+// OneTimeNetbootStatus holds the state of a specific job.bmc.tinkerbell.org object created.
+type OneTimeNetbootStatus struct {
+	// UID is the UID of the job.bmc.tinkerbell.org object associated with this workflow.
+	// This is used to uniquely identify the job.bmc.tinkerbell.org object, as
+	// all objects for a specific Hardware/Machine.bmc.tinkerbell.org are created with the same name.
+	UID types.UID `json:"uid,omitempty"`
+
+	// Complete indicates whether the created job.bmc.tinkerbell.org has reported its conditions as complete.
+	Complete bool `json:"complete,omitempty"`
+
+	// ExistingJobDeleted indicates whether any existing job.bmc.tinkerbell.org was deleted.
+	// The name of each job.bmc.tinkerbell.org object created by the controller is the same, so only one can exist at a time.
+	// Using the same name was chosen so that there is only ever 1 job.bmc.tinkerbell.org per Hardware/Machine.bmc.tinkerbell.org.
+	// This makes clean up easier and we dont just orphan jobs every time.
+	ExistingJobDeleted bool `json:"existingJobDeleted,omitempty"`
+}
+
+// JobCondition describes current state of a job.
+type WorkflowCondition struct {
+	// Type of job condition, Complete or Failed.
+	Type WorkflowConditionType `json:"type" protobuf:"bytes,1,opt,name=type,casttype=WorkflowConditionType"`
+	// Status of the condition, one of True, False, Unknown.
+	Status metav1.ConditionStatus `json:"status" protobuf:"bytes,2,opt,name=status,casttype=k8s.io/api/core/v1.ConditionStatus"`
+	// Reason is a (brief) reason for the condition's last transition.
+	// +optional
+	Reason string `json:"reason,omitempty" protobuf:"bytes,5,opt,name=reason"`
+	// Message is a human readable message indicating details about last transition.
+	// +optional
+	Message string `json:"message,omitempty" protobuf:"bytes,6,opt,name=message"`
+	// Time when the condition was created.
+	// +optional
+	Time *metav1.Time `json:"time,omitempty" protobuf:"bytes,7,opt,name=time"`
 }
 
 // Task represents a series of actions to be completed by a worker.
@@ -62,31 +188,31 @@ type Action struct {
 	Message     string            `json:"message,omitempty"`
 }
 
-// +kubebuilder:subresource:status
-// +kubebuilder:object:root=true
-// +kubebuilder:resource:path=workflows,scope=Namespaced,categories=tinkerbell,shortName=wf,singular=workflow
-// +kubebuilder:storageversion
-// +kubebuilder:printcolumn:JSONPath=".spec.templateRef",name=Template,type=string
-// +kubebuilder:printcolumn:JSONPath=".status.state",name=State,type=string
+// HasCondition checks if the cType condition is present with status cStatus on a bmj.
+func (w *WorkflowStatus) HasCondition(wct WorkflowConditionType, cs metav1.ConditionStatus) bool {
+	for _, c := range w.Conditions {
+		if c.Type == wct {
+			return c.Status == cs
+		}
+	}
 
-// Workflow is the Schema for the Workflows API.
-type Workflow struct {
-	metav1.TypeMeta   `json:",inline"`
-	metav1.ObjectMeta `json:"metadata,omitempty"`
-
-	Spec   WorkflowSpec   `json:"spec,omitempty"`
-	Status WorkflowStatus `json:"status,omitempty"`
+	return false
 }
 
-// +kubebuilder:object:root=true
-
-// WorkflowList contains a list of Workflows.
-type WorkflowList struct {
-	metav1.TypeMeta `json:",inline"`
-	metav1.ListMeta `json:"metadata,omitempty"`
-	Items           []Workflow `json:"items"`
-}
-
-func init() {
-	SchemeBuilder.Register(&Workflow{}, &WorkflowList{})
+// SetCondition updates conditions. If the condition already exists, it updates it.
+// If the condition doesn't exist then it appends the new one (wc).
+func (w *WorkflowStatus) SetCondition(wc WorkflowCondition) {
+	index := -1
+	for i, c := range w.Conditions {
+		if c.Type == wc.Type {
+			index = i
+			break
+		}
+	}
+	if index != -1 {
+		w.Conditions[index] = wc
+		return
+	}
+
+	w.Conditions = append(w.Conditions, wc)
 }