Merge branch 'main' into fix-create-device-message

.github/workflows/fork.yaml

@@ -25,7 +25,7 @@ jobs:
         echo "${new_version}" >> $GITHUB_ENV
         echo 'EOF' >> $GITHUB_ENV
     - name: Create PR
-      if: ${{ env.NEW_VERSION != "" }}
+      if: ${{ env.NEW_VERSION != '' }}
       uses: peter-evans/create-pull-request@v3
       id: cpr
       with:

api/services/microvm/v1alpha1/microvms.swagger.json

@@ -146,7 +146,8 @@
       "enum": [
         "PENDING",
         "CREATED",
-        "FAILED"
+        "FAILED",
+        "DELETING"
       ],
       "default": "PENDING"
     },

api/types/microvm.proto

@@ -150,6 +150,7 @@ message MicroVMStatus {
     PENDING = 0;
     CREATED = 1;
     FAILED = 2;
+    DELETING = 3;
   }
 
   // State stores information about the last known state of the vm and the spec.

core/application/app_test.go

@@ -204,6 +204,7 @@ func TestApp_DeleteMicroVM(t *testing.T) {
 
 				expectedUpdatedSpec := createTestSpec("id1234", "default")
 				expectedUpdatedSpec.Spec.DeletedAt = frozenTime().Unix()
+				expectedUpdatedSpec.Status.State = models.DeletingState
 
 				rm.Save(
 					gomock.AssignableToTypeOf(context.Background()),

core/application/commands.go

@@ -97,6 +97,7 @@ func (a *app) DeleteMicroVM(ctx context.Context, id, namespace string) error {
 	// Set the timestamp when the VMspec was deleted.
 	foundMvm.Spec.DeletedAt = a.ports.Clock().Unix()
 	foundMvm.Status.Retry = 0
+	foundMvm.Status.State = models.DeletingState
 
 	_, err = a.ports.Repo.Save(ctx, foundMvm)
 	if err != nil {

core/application/reconcile.go

@@ -118,20 +118,18 @@ func (a *app) reconcile(ctx context.Context, spec *models.MicroVM, logger *logru
 	localLogger := logger.WithField("vmid", spec.ID.String())
 	localLogger.Info("Starting reconciliation")
 
-	if spec.Status.Retry > a.cfg.MaximumRetry {
-		spec.Status.State = models.FailedState
+	plan := a.plan(spec, localLogger)
 
+	if spec.Status.Retry > a.cfg.MaximumRetry {
 		logger.Error(reachedMaximumRetryError{vmid: spec.ID, retries: spec.Status.Retry})
 
-		return nil
+		return a.saveState(ctx, spec, plan, models.FailedState)
 	}
 
 	if spec.Status.NotBefore > 0 && time.Now().Before(time.Unix(spec.Status.NotBefore, 0)) {
 		return nil
 	}
 
-	plan := a.plan(spec, localLogger)
-
 	execCtx := portsctx.WithPorts(ctx, a.ports)
 
 	executionID, err := a.ports.IdentifierService.GenerateRandom()
@@ -165,6 +163,12 @@ func (a *app) reconcile(ctx context.Context, spec *models.MicroVM, logger *logru
 	spec.Status.Retry = 0
 	spec.Status.NotBefore = 0
 
+	return a.saveState(ctx, spec, plan, models.CreatedState)
+}
+
+func (a *app) saveState(ctx context.Context, spec *models.MicroVM, plan planner.Plan, state models.MicroVMState) error {
+	plan.Finalise(state)
+
 	if _, err := a.ports.Repo.Save(ctx, spec); err != nil {
 		return fmt.Errorf("saving spec after plan execution: %w", err)
 	}

core/models/microvm.go

@@ -1,11 +1,14 @@
 package models
 
+// This state represents the state of the entire Flintlock MVM.
+// The state for the Firecracker MVM itself is represented in ports.MicroVMState.
 type MicroVMState string
 
 const (
-	PendingState = "pending"
-	CreatedState = "created"
-	FailedState  = "failed"
+	PendingState  = "pending"
+	CreatedState  = "created"
+	FailedState   = "failed"
+	DeletingState = "deleting"
 )
 
 // MicroVM represents a microvm machine that is created via a provider.

core/plans/microvm_create_update.go

@@ -82,8 +82,8 @@ func (p *microvmCreateOrUpdatePlan) Create(ctx context.Context) ([]planner.Proce
 	return p.steps, nil
 }
 
-func (p *microvmCreateOrUpdatePlan) Result() interface{} {
-	return nil
+func (p *microvmCreateOrUpdatePlan) Finalise(state models.MicroVMState) {
+	p.vm.Status.State = state
 }
 
 // This is the most important function in the codebase DO NOT REMOVE
@@ -184,9 +184,7 @@ func (p *microvmCreateOrUpdatePlan) ensureStatus() {
 		p.vm.Status.NetworkInterfaces = models.NetworkInterfaceStatuses{}
 	}
 
-	// I'll leave this condition here for safety. If (for some reason) it's
-	// called on a vm that's not pending, leave the status as it is.
-	if p.vm.Status.State == models.PendingState {
-		p.vm.Status.State = models.CreatedState
-	}
+	// If we are going through the create/update steps, then switch to pending first.
+	// When all is done and successful it will be put to created.
+	p.vm.Status.State = models.PendingState
 }

core/plans/microvm_create_update_test.go

@@ -18,13 +18,14 @@ func TestMicroVMCreateOrUpdatePlan(t *testing.T) {
 	mockCtrl := gomock.NewController(t)
 	defer mockCtrl.Finish()
 
+	testVM := createTestSpec("vmid", "namespace")
 	mList, mockedPorts := fakePorts(mockCtrl)
 	ctx := portsctx.WithPorts(
 		context.Background(),
 		mockedPorts,
 	)
 	plan := plans.MicroVMCreateOrUpdatePlan(&plans.CreateOrUpdatePlanInput{
-		VM:             createTestSpec("vmid", "namespace"),
+		VM:             testVM,
 		StateDirectory: "/tmp/path/to/vm",
 	})
 
@@ -105,6 +106,8 @@ func TestMicroVMCreateOrUpdatePlan(t *testing.T) {
 	Expect(createErr).NotTo(HaveOccurred())
 	Expect(steps).To(HaveLen(7))
 
+	Expect(testVM.Status.State).To(Equal(models.MicroVMState(models.PendingState)))
+
 	for _, step := range steps {
 		should, err := step.ShouldDo(ctx)
 
@@ -119,3 +122,31 @@ func TestMicroVMCreateOrUpdatePlan(t *testing.T) {
 		}
 	}
 }
+
+func TestMicroVMPlanFinalise(t *testing.T) {
+	tt := []struct {
+		name  string
+		state models.MicroVMState
+	}{
+		{
+			name:  "finalise with created updates mvm state to created",
+			state: models.CreatedState,
+		},
+		{
+			name:  "finalise with failed updates mvm state to created",
+			state: models.FailedState,
+		},
+	}
+	for _, tc := range tt {
+		RegisterTestingT(t)
+		vm := createTestSpec("vmid", "namespace")
+		plan := plans.MicroVMCreateOrUpdatePlan(&plans.CreateOrUpdatePlanInput{
+			VM:             vm,
+			StateDirectory: "/tmp/path/to/vm",
+		})
+
+		plan.Finalise(tc.state)
+
+		Expect(vm.Status.State).To(Equal(models.MicroVMState(tc.state)))
+	}
+}

core/plans/microvm_delete.go

@@ -93,9 +93,7 @@ func (p *microvmDeletePlan) Create(ctx context.Context) ([]planner.Procedure, er
 	return p.steps, nil
 }
 
-// Result is the result of the plan.
-func (p *microvmDeletePlan) Result() interface{} {
-	return nil
+func (p *microvmDeletePlan) Finalise(_ models.MicroVMState) {
 }
 
 // This is the most important function in the codebase DO NOT REMOVE

core/ports/services.go

@@ -29,6 +29,8 @@ type MicroVMService interface {
 	State(ctx context.Context, id string) (MicroVMState, error)
 }
 
+// This state represents the state of the Firecracker MVM process itself
+// The state for the entire Flintlock MVM is represented in models.MicroVMState.
 type MicroVMState string
 
 const (

go.mod

@@ -33,10 +33,10 @@ require (
 	github.com/spf13/afero v1.6.0
 	github.com/spf13/cobra v1.2.1
 	github.com/spf13/pflag v1.0.5
-	github.com/spf13/viper v1.9.0
+	github.com/spf13/viper v1.10.0
 	github.com/vishvananda/netlink v1.1.1-0.20201029203352-d40f9887b852
 	github.com/weaveworks/flintlock/api v0.0.0-00010101000000-000000000000
-	golang.org/x/net v0.0.0-20210614182718-04defd469f4e // indirect
+	golang.org/x/net v0.0.0-20210813160813-60bc85c4be6d // indirect
 	google.golang.org/grpc v1.42.0
 	google.golang.org/protobuf v1.27.1
 	gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b
@@ -51,7 +51,7 @@ require (
 	github.com/asaskevich/govalidator v0.0.0-20200907205600-7a23bdc65eef // indirect
 	github.com/beorn7/perks v1.0.1 // indirect
 	github.com/bits-and-blooms/bitset v1.2.0 // indirect
-	github.com/cespare/xxhash/v2 v2.1.1 // indirect
+	github.com/cespare/xxhash/v2 v2.1.2 // indirect
 	github.com/containerd/cgroups v1.0.1 // indirect
 	github.com/containerd/continuity v0.1.0 // indirect
 	github.com/containerd/fifo v1.0.0 // indirect
@@ -75,7 +75,7 @@ require (
 	github.com/gofrs/uuid v3.3.0+incompatible // indirect
 	github.com/gogo/googleapis v1.4.0 // indirect
 	github.com/gogo/protobuf v1.3.2 // indirect
-	github.com/golang/groupcache v0.0.0-20200121045136-8c9f03a8e57e // indirect
+	github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
 	github.com/golang/protobuf v1.5.2 // indirect
 	github.com/hashicorp/errwrap v1.0.0 // indirect
 	github.com/hashicorp/go-multierror v1.1.0 // indirect
@@ -86,7 +86,7 @@ require (
 	github.com/magiconair/properties v1.8.5 // indirect
 	github.com/mailru/easyjson v0.7.1 // indirect
 	github.com/matttproud/golang_protobuf_extensions v1.0.2-0.20181231171920-c182affec369 // indirect
-	github.com/mitchellh/mapstructure v1.4.2 // indirect
+	github.com/mitchellh/mapstructure v1.4.3 // indirect
 	github.com/moby/locker v1.0.1 // indirect
 	github.com/moby/sys/mountinfo v0.4.1 // indirect
 	github.com/nxadm/tail v1.4.8 // indirect
@@ -105,10 +105,10 @@ require (
 	go.opencensus.io v0.23.0 // indirect
 	golang.org/x/crypto v0.0.0-20210817164053-32db794688a5 // indirect
 	golang.org/x/sync v0.0.0-20210220032951-036812b2e83c // indirect
-	golang.org/x/sys v0.0.0-20210823070655-63515b42dcdf // indirect
-	golang.org/x/text v0.3.6 // indirect
-	google.golang.org/genproto v0.0.0-20211129164237-f09f9a12af12 // indirect
-	gopkg.in/ini.v1 v1.63.2 // indirect
+	golang.org/x/sys v0.0.0-20211205182925-97ca703d548d // indirect
+	golang.org/x/text v0.3.7 // indirect
+	google.golang.org/genproto v0.0.0-20211208223120-3a66f561d7aa // indirect
+	gopkg.in/ini.v1 v1.66.2 // indirect
 	gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 // indirect
 	gopkg.in/yaml.v2 v2.4.0 // indirect
 )