Add an 'id' to plan command status

Adding an identifier to plan command status fixes a problem in signal node reconciliation where multiple commands of the same name (ie. k0supdate + k0supdate) wouldn't be possible. This also removes the redundant 'CommandUpdateItem' struct in favor for embedding the 'k0s' and 'airgap' update structs into the 'Command' struct. This aligns better with the CRD structures. This is a breaking change. Fixes k0sproject#227 Signed-off-by: Shane Jarych <sjarych@mirantis.com>
s0j · Jun 24, 2022 · 0d9f93f · 0d9f93f
1 parent cae8e04
commit 0d9f93f
Show file tree

Hide file tree

Showing 25 changed files with 288 additions and 302 deletions.
diff --git a/...lot/embedded/manifests/autopilot.k0sproject.io/v1beta2/autopilot.k0sproject.io_plans.yaml b/...lot/embedded/manifests/autopilot.k0sproject.io/v1beta2/autopilot.k0sproject.io_plans.yaml
@@ -55,21 +55,21 @@ spec:
                       properties:
                         platforms:
                           additionalProperties:
-                            description: PlanResourceUrl is a remote Url resource.
+                            description: PlanResourceURL is a remote URL resource.
                             properties:
                               sha256:
                                 description: Sha256 provides an optional SHA256 hash
-                                  of the Url's content for verification.
+                                  of the URL's content for verification.
                                 type: string
                               url:
-                                description: Url is the Url of a downloadable resource.
+                                description: URL is the URL of a downloadable resource.
                                 type: string
                             required:
                             - url
                             type: object
                           description: Platforms is a map of PlanResourceUrls to platform
                             identifiers, allowing a single k0s version to have multiple
-                            Url resources based on platform.
+                            URL resources based on platform.
                           type: object
                         targets:
                           description: Targets defines how the controllers/workers
@@ -209,6 +209,10 @@ spec:
                 items:
                   description: PlanCommandStatus is the status of a known command.
                   properties:
+                    id:
+                      description: Id is a unique identifier for this command in a
+                        Plan
+                      type: integer
                     k0supdate:
                       description: K0sUpdate is the status of the `K0sUpdate` command.
                       properties:
@@ -267,6 +271,7 @@ spec:
                       description: State is the current state of the plan command.
                       type: string
                   required:
+                  - id
                   - state
                   type: object
                 type: array

diff --git a/...dded/manifests/autopilot.k0sproject.io/v1beta2/autopilot.k0sproject.io_updateconfigs.yaml b/...dded/manifests/autopilot.k0sproject.io/v1beta2/autopilot.k0sproject.io_updateconfigs.yaml
@@ -52,22 +52,22 @@ spec:
                           properties:
                             platforms:
                               additionalProperties:
-                                description: PlanResourceUrl is a remote Url resource.
+                                description: PlanResourceURL is a remote URL resource.
                                 properties:
                                   sha256:
                                     description: Sha256 provides an optional SHA256
-                                      hash of the Url's content for verification.
+                                      hash of the URL's content for verification.
                                     type: string
                                   url:
-                                    description: Url is the Url of a downloadable
+                                    description: URL is the URL of a downloadable
                                       resource.
                                     type: string
                                 required:
                                 - url
                                 type: object
                               description: Platforms is a map of PlanResourceUrls
                                 to platform identifiers, allowing a single k0s version
-                                to have multiple Url resources based on platform.
+                                to have multiple URL resources based on platform.
                               type: object
                             targets:
                               description: Targets defines how the controllers/workers

diff --git a/autopilot/pkg/apis/autopilot.k0sproject.io/v1beta2/types.go b/autopilot/pkg/apis/autopilot.k0sproject.io/v1beta2/types.go
@@ -212,6 +212,9 @@ type PlanStatus struct {
 
 // PlanCommandStatus is the status of a known command.
 type PlanCommandStatus struct {
+	// Id is a unique identifier for this command in a Plan
+	Id int `json:"id"`
+
 	// State is the current state of the plan command.
 	State PlanStateType `json:"state"`
 

diff --git a/autopilot/pkg/controller/plans/cmdprovider/k0supdate/schedulable.go b/autopilot/pkg/controller/plans/cmdprovider/k0supdate/schedulable.go
@@ -89,7 +89,7 @@ func (kp *k0supdate) Schedulable(ctx context.Context, cmd apv1beta2.PlanCommand,
 	// This has the possibility of ending reconciliation early if the node and plan platforms
 	// disagree. This target state will move to `IncompleteTargets` in this case.
 
-	nodeCopy, err := signalNodeUpdate(signalNodeDelegate.DeepCopy(signalNode), cmd.K0sUpdate, status.K0sUpdate)
+	nodeCopy, err := signalNodeUpdate(signalNodeDelegate.DeepCopy(signalNode), cmd, status)
 	if err != nil {
 		logger.Warnf("Unable to update signal node: %v", err)
 		return appc.PlanIncompleteTargets, false, nil
@@ -170,30 +170,33 @@ func findNextPendingRandom(nodes []apv1beta2.PlanCommandTargetStatus) (*apv1beta
 }
 
 // signalNodeUpdate builds a signalling update request, and adds it to the provided node
-func signalNodeUpdate(node crcli.Object, cmd *apv1beta2.PlanCommandK0sUpdate, cmdStatus *apv1beta2.PlanCommandK0sUpdateStatus) (crcli.Object, error) {
+func signalNodeUpdate(node crcli.Object, cmd apv1beta2.PlanCommand, cmdStatus *apv1beta2.PlanCommandStatus) (crcli.Object, error) {
+	if cmdStatus == nil || cmd.K0sUpdate == nil || cmdStatus.K0sUpdate == nil {
+		return nil, fmt.Errorf("invalid plan command arguments for k0supdate")
+	}
+
 	// Determine the platform identifier of the target signal node
 	nodePlatformID, err := signalNodePlatformIdentifier(node)
 	if err != nil {
-		updatePlanCommandTargetStatusByName(node.GetName(), appc.SignalMissingPlatform, cmdStatus)
+		updatePlanCommandTargetStatusByName(node.GetName(), appc.SignalMissingPlatform, cmdStatus.K0sUpdate)
 		return nil, err
 	}
 
 	// Find the appropriate update content for this signal node
-	updateContent, updateContentOk := cmd.Platforms[nodePlatformID]
+	updateContent, updateContentOk := cmd.K0sUpdate.Platforms[nodePlatformID]
 	if !updateContentOk {
-		updatePlanCommandTargetStatusByName(node.GetName(), appc.SignalMissingPlatform, cmdStatus)
+		updatePlanCommandTargetStatusByName(node.GetName(), appc.SignalMissingPlatform, cmdStatus.K0sUpdate)
 		return nil, err
 	}
 
 	signalData := apsigv2.SignalData{
 		Created: time.Now().Format(time.RFC3339),
 		Command: apsigv2.Command{
-			Update: &apsigv2.CommandUpdateItem{
-				K0s: &apsigv2.CommandUpdateItemK0s{
-					URL:     updateContent.URL,
-					Version: cmd.Version,
-					Sha256:  updateContent.Sha256,
-				},
+			ID: &cmdStatus.Id,
+			K0sUpdate: &apsigv2.CommandK0sUpdate{
+				URL:     updateContent.URL,
+				Version: cmd.K0sUpdate.Version,
+				Sha256:  updateContent.Sha256,
 			},
 		},
 	}

diff --git a/autopilot/pkg/controller/plans/cmdprovider/k0supdate/schedulable_test.go b/autopilot/pkg/controller/plans/cmdprovider/k0supdate/schedulable_test.go
@@ -138,6 +138,7 @@ func TestSchedulable(t *testing.T) {
 				},
 			},
 			apv1beta2.PlanCommandStatus{
+				Id:    123,
 				State: appc.PlanSchedulable,
 				K0sUpdate: &apv1beta2.PlanCommandK0sUpdateStatus{
 					Controllers: []apv1beta2.PlanCommandTargetStatus{

diff --git a/autopilot/pkg/controller/plans/cmdprovider/k0supdate/schedulablewait.go b/autopilot/pkg/controller/plans/cmdprovider/k0supdate/schedulablewait.go
@@ -130,16 +130,10 @@ func (kp *k0supdate) reconcileSignalNodeStatusTarget(ctx context.Context, cmd ap
 // isSignalDataSameCommand determines if the `PlanCommand` and the command specified in the signal data represent
 // the same command.
 func isSignalDataSameCommand(cmd apv1beta2.PlanCommand, signalData apsigv2.SignalData) bool {
-	update := signalData.Command.Update
-	if update == nil {
-		return false
-	}
-
 	// As additional commands are implemented, they will need to be reflected here.
-
 	switch {
 	case cmd.K0sUpdate != nil:
-		return update.K0s != nil
+		return signalData.Command.K0sUpdate != nil
 	}
 
 	return false

diff --git a/autopilot/pkg/controller/plans/cmdprovider/k0supdate/schedulablewait_test.go b/autopilot/pkg/controller/plans/cmdprovider/k0supdate/schedulablewait_test.go
@@ -279,11 +279,10 @@ func TestSchedulableWait(t *testing.T) {
 							apsigv2.SignalData{
 								Created: "now",
 								Command: apsigv2.Command{
-									Update: &apsigv2.CommandUpdateItem{
-										K0s: &apsigv2.CommandUpdateItemK0s{
-											URL:     "https://foo.bar.baz/download.tar.gz",
-											Version: "v1.2.3",
-										},
+									ID: new(int),
+									K0sUpdate: &apsigv2.CommandK0sUpdate{
+										URL:     "https://foo.bar.baz/download.tar.gz",
+										Version: "v1.2.3",
 									},
 								},
 								Status: &apsigv2.Status{
@@ -460,9 +459,8 @@ func TestIsSignalDataSameCommand(t *testing.T) {
 			},
 			apsigv2.SignalData{
 				Command: apsigv2.Command{
-					Update: &apsigv2.CommandUpdateItem{
-						K0s: &apsigv2.CommandUpdateItemK0s{},
-					},
+					ID:        new(int),
+					K0sUpdate: &apsigv2.CommandK0sUpdate{},
 				},
 			},
 			true,
@@ -482,9 +480,8 @@ func TestIsSignalDataSameCommand(t *testing.T) {
 			apv1beta2.PlanCommand{},
 			apsigv2.SignalData{
 				Command: apsigv2.Command{
-					Update: &apsigv2.CommandUpdateItem{
-						K0s: &apsigv2.CommandUpdateItemK0s{},
-					},
+					ID:        new(int),
+					K0sUpdate: &apsigv2.CommandK0sUpdate{},
 				},
 			},
 			false,
@@ -496,9 +493,8 @@ func TestIsSignalDataSameCommand(t *testing.T) {
 			},
 			apsigv2.SignalData{
 				Command: apsigv2.Command{
-					Update: &apsigv2.CommandUpdateItem{
-						Airgap: &apsigv2.CommandUpdateItemAirgap{},
-					},
+					ID:           new(int),
+					AirgapUpdate: &apsigv2.CommandAirgapUpdate{},
 				},
 			},
 			false,

diff --git a/autopilot/pkg/controller/plans/core/planstatehandler.go b/autopilot/pkg/controller/plans/core/planstatehandler.go
@@ -127,7 +127,7 @@ func findPlanCommandStatus(status *apv1beta2.PlanStatus, idx int) *apv1beta2.Pla
 	}
 
 	// .. otherwise, add a new one and return it
-	status.Commands = append(status.Commands, apv1beta2.PlanCommandStatus{})
+	status.Commands = append(status.Commands, apv1beta2.PlanCommandStatus{Id: idx})
 
 	return &status.Commands[len(status.Commands)-1]
 }
@@ -142,7 +142,9 @@ func (h *planStateHandler) planCommandProviderLookup(cmd apv1beta2.PlanCommand)
 	rpcmd := reflect.Indirect(reflect.ValueOf(cmd))
 
 	for i := 0; i < rpcmd.NumField(); i++ {
-		if v := rpcmd.Field(i); !v.IsNil() {
+		v := rpcmd.Field(i)
+
+		if v.Kind() == reflect.Pointer && !v.IsNil() {
 			fieldName := rpcmd.Type().Field(i).Name
 			if handler, found := h.commandProviderMap[fieldName]; found {
 				return fieldName, handler, true

diff --git a/autopilot/pkg/controller/signal/airgap/download.go b/autopilot/pkg/controller/signal/airgap/download.go
@@ -59,8 +59,8 @@ func registerDownloadController(logger *logrus.Entry, mgr crman.Manager, eventFi
 func (b downloadManfiestBuilderAirgap) Build(signalNode crcli.Object, signalData apsigv2.SignalData) (apsigcomm.DownloadManifest, error) {
 	m := apsigcomm.DownloadManifest{
 		Config: apdl.Config{
-			URL:          signalData.Command.Update.Airgap.URL,
-			ExpectedHash: signalData.Command.Update.Airgap.Sha256,
+			URL:          signalData.Command.AirgapUpdate.URL,
+			ExpectedHash: signalData.Command.AirgapUpdate.Sha256,
 			Hasher:       sha256.New(),
 			DownloadDir:  path.Join(b.k0sDataDir, apconst.K0sManifestSubDir),
 		},

diff --git a/autopilot/pkg/controller/signal/airgap/init.go b/autopilot/pkg/controller/signal/airgap/init.go
@@ -56,6 +56,6 @@ func RegisterControllers(ctx context.Context, logger *logrus.Entry, mgr crman.Ma
 // provided SignalData is an 'airgap' update.
 func signalDataUpdateCommandAirgapPredicate() apsigpred.SignalDataPredicate {
 	return func(signalData apsigv2.SignalData) bool {
-		return signalData.Command.Update != nil && signalData.Command.Update.Airgap != nil
+		return signalData.Command.AirgapUpdate != nil
 	}
 }
diff --git a/autopilot/pkg/controller/signal/airgap/init_test.go b/autopilot/pkg/controller/signal/airgap/init_test.go
@@ -34,9 +34,8 @@ func TestSignalDataUpdateCommandAirgapPredicate(t *testing.T) {
 			"Found",
 			apsigv2.SignalData{
 				Command: apsigv2.Command{
-					Update: &apsigv2.CommandUpdateItem{
-						Airgap: &apsigv2.CommandUpdateItemAirgap{},
-					},
+					ID:           new(int),
+					AirgapUpdate: &apsigv2.CommandAirgapUpdate{},
 				},
 			},
 			true,
@@ -45,9 +44,8 @@ func TestSignalDataUpdateCommandAirgapPredicate(t *testing.T) {
 			"NotFoundK0s",
 			apsigv2.SignalData{
 				Command: apsigv2.Command{
-					Update: &apsigv2.CommandUpdateItem{
-						K0s: &apsigv2.CommandUpdateItemK0s{},
-					},
+					ID:        new(int),
+					K0sUpdate: &apsigv2.CommandK0sUpdate{},
 				},
 			},
 			false,

diff --git a/autopilot/pkg/controller/signal/airgap/signal_test.go b/autopilot/pkg/controller/signal/airgap/signal_test.go
@@ -67,13 +67,12 @@ func TestSignalControllerEventFilter(t *testing.T) {
 									"planId":"abc123",
 									"created":"now",
 									"command": {
-										"update": {
-											"airgap": {
-												"version": "v1.2.3",
-												"url": "https://www.google.com/download.tar.gz",
-												"timestamp": "2021-10-20T19:06:56Z",
-												"sha256": "thisisthesha"
-											}
+										"id": 123,
+										"airgapupdate": {
+											"version": "v1.2.3",
+											"url": "https://www.google.com/download.tar.gz",
+											"timestamp": "2021-10-20T19:06:56Z",
+											"sha256": "thisisthesha"
 										}
 									}
 								}
@@ -106,13 +105,12 @@ func TestSignalControllerEventFilter(t *testing.T) {
 									"planId":"abc123",
 									"created":"now",
 									"command": {
-										"update": {
-											"k0s": {
-												"version": "v1.2.3",
-												"url": "https://www.google.com/download.tar.gz",
-												"timestamp": "2021-10-20T19:06:56Z",
-												"sha256": "thisisthesha"
-											}
+										"id": 123,
+										"k0supdate": {
+											"version": "v1.2.3",
+											"url": "https://www.google.com/download.tar.gz",
+											"timestamp": "2021-10-20T19:06:56Z",
+											"sha256": "thisisthesha"
 										}
 									},
 									"status": {
@@ -139,13 +137,12 @@ func TestSignalControllerEventFilter(t *testing.T) {
 									"planId":"abc123",
 									"created":"now",
 									"command": {
-										"update": {
-											"k0s": {
-												"version": "v1.2.3",
-												"url": "https://www.google.com/download.tar.gz",
-												"timestamp": "2021-10-20T19:06:56Z",
-												"sha256": "thisisthesha"
-											}
+										"id": 123,
+										"k0supdate": {
+											"version": "v1.2.3",
+											"url": "https://www.google.com/download.tar.gz",
+											"timestamp": "2021-10-20T19:06:56Z",
+											"sha256": "thisisthesha"
 										}
 									}
 								}
@@ -167,13 +164,12 @@ func TestSignalControllerEventFilter(t *testing.T) {
 									"planId":"abc123",
 									"created":"now",
 									"command": {
-										"update": {
-											"k0s": {
-												"version": "v1.2.3",
-												"url": "https://www.google.com/download.tar.gz",
-												"timestamp": "2021-10-20T19:06:56Z",
-												"sha256": "thisisthesha"
-											}
+										"id": 123,
+										"k0supdate": {
+											"version": "v1.2.3",
+											"url": "https://www.google.com/download.tar.gz",
+											"timestamp": "2021-10-20T19:06:56Z",
+											"sha256": "thisisthesha"
 										}
 									}
 								}
@@ -206,13 +202,12 @@ func TestSignalControllerEventFilter(t *testing.T) {
 									"planId":"abc123",
 									"created":"now",
 									"command": {
-										"update": {
-											"k0s": {
-												"version": "v1.2.3",
-												"url": "https://www.google.com/download.tar.gz",
-												"timestamp": "2021-10-20T19:06:56Z",
-												"sha256": "thisisthesha"
-											}
+										"id": 123,
+										"k0supdate": {
+											"version": "v1.2.3",
+											"url": "https://www.google.com/download.tar.gz",
+											"timestamp": "2021-10-20T19:06:56Z",
+											"sha256": "thisisthesha"
 										}
 									}
 								}
@@ -249,12 +244,11 @@ func TestHandle(t *testing.T) {
 					"planId":"abc123",
 					"created":"now",
 					"command": {
-						"update": {
-							"airgap": {
-								"version": "v1.23.3+k0s.1",
-								"url": "https://github.com/k0sproject/k0s/releases/download/v1.23.3%2Bk0s.1/k0s-airgap-bundle-v1.23.3+k0s.1-amd64",
-								"sha256": "258f3edd0c260a23c579406f5cc04a599a6f59cc1707f9bd523d7a9abc07f0e2"
-							}
+						"id": 123,
+						"airgapupdate": {
+							"version": "v1.23.3+k0s.1",
+							"url": "https://github.com/k0sproject/k0s/releases/download/v1.23.3%2Bk0s.1/k0s-airgap-bundle-v1.23.3+k0s.1-amd64",
+							"sha256": "258f3edd0c260a23c579406f5cc04a599a6f59cc1707f9bd523d7a9abc07f0e2"
 						}
 					}
 				}