Merge pull request #3 from miscord-dev/fix-bugs

Fix bugs

Author: tsuzu

config/crd/kustomization.yaml

@@ -8,6 +8,9 @@ resources:
 - bases/infrastructure.cluster.x-k8s.io_incusmachinetemplates.yaml
 # +kubebuilder:scaffold:crdkustomizeresource
 
+commonLabels:
+  cluster.x-k8s.io/v1beta1: v1alpha1
+
 patches:
 # [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix.
 # patches here are for enabling the conversion webhook for each CRD

config/default/kustomization.yaml

@@ -1,5 +1,5 @@
 # Adds namespace to all resources.
-namespace: cluster-api-provider-incus-system
+namespace: capincus-system
 
 # Value of this field is prepended to the
 # names of all resources, e.g. a deployment named

config/rbac/role.yaml

@@ -4,11 +4,20 @@ kind: ClusterRole
 metadata:
   name: manager-role
 rules:
+- apiGroups:
+  - ""
+  resources:
+  - secrets
+  verbs:
+  - get
+  - list
+  - watch
 - apiGroups:
   - cluster.x-k8s.io
   resources:
   - clusters
   - machines
+  - machinesets
   verbs:
   - get
   - list

go.mod

@@ -3,6 +3,7 @@ module github.com/miscord-dev/cluster-api-provider-incus
 go 1.22.0
 
 require (
+	github.com/google/uuid v1.6.0
 	github.com/lxc/incus v0.7.0
 	github.com/onsi/ginkgo/v2 v2.19.1
 	github.com/onsi/gomega v1.34.0
@@ -43,7 +44,6 @@ require (
 	github.com/google/go-cmp v0.6.0 // indirect
 	github.com/google/gofuzz v1.2.0 // indirect
 	github.com/google/pprof v0.0.0-20240525223248-4bfdf5a9a2af // indirect
-	github.com/google/uuid v1.6.0 // indirect
 	github.com/gorilla/schema v1.2.1 // indirect
 	github.com/gorilla/securecookie v1.1.2 // indirect
 	github.com/gorilla/websocket v1.5.1 // indirect

internal/controller/incusmachine_controller.go

@@ -20,7 +20,9 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"time"
 
+	"github.com/google/uuid"
 	"github.com/lxc/incus/shared/api"
 	infrav1alpha1 "github.com/miscord-dev/cluster-api-provider-incus/api/v1alpha1"
 	"github.com/miscord-dev/cluster-api-provider-incus/pkg/incus"
@@ -55,6 +57,8 @@ type IncusMachineReconciler struct {
 // +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=incusmachines/finalizers,verbs=update
 // +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=clusters,verbs=get;list;watch
 // +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=machines,verbs=get;list;watch
+// +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=machinesets,verbs=get;list;watch
+// +kubebuilder:rbac:groups=core,resources=secrets,verbs=get;list;watch
 
 // Reconcile is part of the main kubernetes reconciliation loop which aims to
 // move the current state of the cluster closer to the desired state.
@@ -139,6 +143,13 @@ func (r *IncusMachineReconciler) Reconcile(ctx context.Context, req ctrl.Request
 	if err != nil {
 		return ctrl.Result{}, err
 	}
+	deleted := !incusMachine.ObjectMeta.DeletionTimestamp.IsZero()
+	containsFinalizer := controllerutil.ContainsFinalizer(incusMachine, infrav1alpha1.MachineFinalizer)
+
+	if deleted && !containsFinalizer {
+		// Return early since the object is being deleted and doesn't have the finalizer.
+		return ctrl.Result{}, nil
+	}
 	// Always attempt to Patch the IncusMachine object and status after each reconciliation.
 	defer func() {
 		if err := patchIncusMachine(ctx, patchHelper, incusMachine); err != nil {
@@ -149,18 +160,24 @@ func (r *IncusMachineReconciler) Reconcile(ctx context.Context, req ctrl.Request
 		}
 	}()
 
-	// Add finalizer first if not set to avoid the race condition between init and delete.
-	// Note: Finalizers in general can only be added when the deletionTimestamp is not set.
-	if incusMachine.ObjectMeta.DeletionTimestamp.IsZero() && !controllerutil.ContainsFinalizer(incusMachine, infrav1alpha1.MachineFinalizer) {
-		controllerutil.AddFinalizer(incusMachine, infrav1alpha1.MachineFinalizer)
-		return ctrl.Result{}, nil
-	}
+	log.Info("Reconciling IncusMachine")
 
 	// Handle deleted machines
-	if !incusMachine.ObjectMeta.DeletionTimestamp.IsZero() {
+	if deleted {
 		return r.reconcileDelete(ctx, incusCluster, machine, incusMachine)
 	}
 
+	// Add finalizer first if not set to avoid the race condition between init and delete.
+	// Note: Finalizers in general can only be added when the deletionTimestamp is not set.
+	if !containsFinalizer {
+		log.Info("Adding finalizer for IncusMachine")
+
+		controllerutil.AddFinalizer(incusMachine, infrav1alpha1.MachineFinalizer)
+		return ctrl.Result{
+			Requeue: true,
+		}, nil
+	}
+
 	// Handle non-deleted machines
 	return r.reconcileNormal(ctx, cluster, incusCluster, machine, incusMachine)
 }
@@ -198,25 +215,43 @@ func (r *IncusMachineReconciler) reconcileDelete(ctx context.Context, _ *infrav1
 	// 	return err
 	// }
 
-	output, err := r.IncusClient.GetInstance(ctx, incusMachine.Name)
-	if errors.Is(err, incus.ErrorInstanceNotFound) {
+	exists, err := r.IncusClient.InstanceExists(ctx, incusMachine.Name)
+	if err != nil {
+		return ctrl.Result{}, fmt.Errorf("failed to check if instance exists: %w", err)
+	}
+	if !exists {
 		// Instance is already deleted so remove the finalizer.
+		log.Info("Deleting finalizer from IncusMachine")
 		controllerutil.RemoveFinalizer(incusMachine, infrav1alpha1.MachineFinalizer)
 		return ctrl.Result{}, nil
 	}
+
+	output, err := r.IncusClient.GetInstance(ctx, incusMachine.Name)
 	if err != nil {
 		return ctrl.Result{}, fmt.Errorf("failed to get instance: %w", err)
 	}
 
 	if output.StatusCode != api.Stopped &&
 		output.StatusCode != api.Stopping {
+		log.Info("Stopping instance")
+
 		if err := r.IncusClient.StopInstance(ctx, incusMachine.Name); err != nil {
 			log.Info("Failed to stop instance", "error", err)
 		}
+
+		return ctrl.Result{
+			RequeueAfter: 5 * time.Second,
+		}, nil
 	} else if output.StatusCode != api.OperationCreated {
+		log.Info("Deleting instance")
+
 		if err := r.IncusClient.DeleteInstance(ctx, incusMachine.Name); err != nil {
 			return ctrl.Result{}, fmt.Errorf("failed to delete instance: %w", err)
 		}
+
+		return ctrl.Result{
+			RequeueAfter: 5 * time.Second,
+		}, nil
 	}
 
 	return ctrl.Result{
@@ -240,9 +275,20 @@ func (r *IncusMachineReconciler) reconcileNormal(ctx context.Context, cluster *c
 	}
 	dataSecretName := *machine.Spec.Bootstrap.DataSecretName
 
-	_, err := r.IncusClient.GetInstance(ctx, incusMachine.Name)
+	output, err := r.IncusClient.GetInstance(ctx, incusMachine.Name)
 	if err == nil {
-		return ctrl.Result{}, nil
+		if r.isMachineReady(output) {
+			log.Info("IncusMachine instance is ready")
+
+			incusMachine.Spec.ProviderID = &output.ProviderID
+			incusMachine.Status.Ready = true
+
+			return ctrl.Result{}, nil
+		}
+
+		return ctrl.Result{
+			RequeueAfter: 10 * time.Second,
+		}, nil
 	}
 	if !errors.Is(err, incus.ErrorInstanceNotFound) {
 		return ctrl.Result{}, fmt.Errorf("failed to get instance: %w", err)
@@ -253,20 +299,32 @@ func (r *IncusMachineReconciler) reconcileNormal(ctx context.Context, cluster *c
 		return ctrl.Result{}, err
 	}
 
+	log.Info("Creating IncusMachine instance")
+
+	uuid, err := uuid.NewV7()
+	if err != nil {
+		return ctrl.Result{}, fmt.Errorf("failed to generate UUID: %w", err)
+	}
+
+	providerID := fmt.Sprintf("incus://%s", uuid.String())
+
 	// Create the instance
 	err = r.IncusClient.CreateInstance(ctx, incus.CreateInstanceInput{
 		Name: incusMachine.Name,
 		BootstrapData: incus.BootstrapData{
 			Data:   bootstrapData,
 			Format: string(bootstrapFormat),
 		},
+		ProviderID:   providerID,
 		InstanceSpec: incusMachine.Spec.InstanceSpec,
 	})
 	if err != nil {
 		return ctrl.Result{}, fmt.Errorf("failed to create instance: %w", err)
 	}
 
-	return ctrl.Result{}, nil
+	return ctrl.Result{
+		RequeueAfter: 10 * time.Second,
+	}, nil
 }
 
 func (r *IncusMachineReconciler) getBootstrapData(ctx context.Context, namespace string, dataSecretName string) (string, bootstrapv1.Format, error) {
@@ -289,6 +347,10 @@ func (r *IncusMachineReconciler) getBootstrapData(ctx context.Context, namespace
 	return string(value), bootstrapv1.Format(format), nil
 }
 
+func (r *IncusMachineReconciler) isMachineReady(output *incus.GetInstanceOutput) bool {
+	return output.StatusCode == api.Running
+}
+
 // SetupWithManager sets up the controller with the Manager.
 func (r *IncusMachineReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager) error {
 	clusterToIncusMachines, err := util.ClusterToTypedObjectsMapper(mgr.GetClient(), &infrav1alpha1.IncusMachineList{}, mgr.GetScheme())

pkg/incus/incus.go

@@ -15,6 +15,12 @@ var (
 	ErrorInstanceNotFound = fmt.Errorf("instance not found")
 )
 
+const (
+	configUserDataKey   = "cloud-init.user-data"
+	configProviderIDKey = "user.provider-id"
+	configMetaDataKey   = "user.meta-data"
+)
+
 type BootstrapData struct {
 	Format string
 	Data   string
@@ -23,6 +29,7 @@ type BootstrapData struct {
 type CreateInstanceInput struct {
 	Name          string
 	BootstrapData BootstrapData
+	ProviderID    string
 
 	infrav1alpha1.InstanceSpec
 }
@@ -31,12 +38,14 @@ type GetInstanceOutput struct {
 	Name string
 	infrav1alpha1.InstanceSpec
 
+	ProviderID string
 	// TODO: Add status
 	StatusCode api.StatusCode
 }
 
 type Client interface {
 	CreateInstance(ctx context.Context, spec CreateInstanceInput) error
+	InstanceExists(ctx context.Context, name string) (bool, error)
 	GetInstance(ctx context.Context, name string) (*GetInstanceOutput, error)
 	DeleteInstance(ctx context.Context, name string) error
 	StopInstance(ctx context.Context, name string) error
@@ -54,21 +63,27 @@ func NewClient(instanceServer incusclient.InstanceServer) Client {
 
 func (c *client) CreateInstance(ctx context.Context, spec CreateInstanceInput) error {
 	config := maps.Clone(spec.Config)
+	if config == nil {
+		config = make(map[string]string)
+	}
 
 	switch spec.BootstrapData.Format {
 	case "cloud-config":
-		config["cloud-init.user-data"] = spec.BootstrapData.Data
+		config[configUserDataKey] = spec.BootstrapData.Data
 	case "":
 		// Do nothing
 	default:
 		return fmt.Errorf("unsupported bootstrap data format: %s", spec.BootstrapData.Format)
 	}
 
+	config[configProviderIDKey] = spec.ProviderID
+	config[configMetaDataKey] = fmt.Sprintf("provider-id: %s", spec.ProviderID)
+
 	req := api.InstancesPost{
 		Name: spec.Name,
 		InstancePut: api.InstancePut{
 			Architecture: spec.Architecture,
-			Config:       spec.Config,
+			Config:       config,
 			Devices:      spec.Devices,
 			Ephemeral:    spec.Ephemeral,
 			Profiles:     spec.Profiles,
@@ -108,6 +123,19 @@ func (c *client) CreateInstance(ctx context.Context, spec CreateInstanceInput) e
 	return nil
 }
 
+func (c *client) InstanceExists(ctx context.Context, name string) (bool, error) {
+	_, _, err := c.client.GetInstance(name)
+	if err != nil {
+		if api.StatusErrorCheck(err, http.StatusNotFound) {
+			return false, nil
+		}
+
+		return false, fmt.Errorf("failed to get instance: %w", err)
+	}
+
+	return true, nil
+}
+
 func (c *client) GetInstance(ctx context.Context, name string) (*GetInstanceOutput, error) {
 	resp, _, err := c.client.GetInstanceFull(name)
 	if err != nil {
@@ -131,6 +159,7 @@ func (c *client) GetInstance(ctx context.Context, name string) (*GetInstanceOutp
 			Description:  resp.Description,
 			Type:         infrav1alpha1.InstanceType(resp.Type),
 		},
+		ProviderID: resp.Config[configProviderIDKey],
 		StatusCode: resp.StatusCode,
 	}, nil
 }

test/e2e/e2e_test.go

@@ -31,7 +31,7 @@ import (
 )
 
 // namespace where the project is deployed in
-const namespace = "cluster-api-provider-incus-system"
+const namespace = "capincus-system"
 
 // serviceAccountName created for the project
 const serviceAccountName = "cluster-api-provider-incus-controller-manager"
@@ -291,6 +291,15 @@ var _ = Describe("Manager", Ordered, func() {
 		//    fmt.Sprintf(`controller_runtime_reconcile_total{controller="%s",result="success"} 1`,
 		//    strings.ToLower(<Kind>),
 		// ))
+
+		It("create an IncusCluster", func() {
+			By("creating a ClusterClass/Cluster/IncusCluster/etc for the IncusCluster")
+			cmd := exec.Command("kubectl", "apply", "-n", namespace,
+				"-f", ".github/cluster.yml",
+			)
+			_, err := utils.Run(cmd)
+			Expect(err).NotTo(HaveOccurred(), "Failed to create Cluster")
+		})
 	})
 })