Skip to content

Commit

Permalink
Fix bugs
Browse files Browse the repository at this point in the history
  • Loading branch information
tsuzu committed Dec 12, 2024
1 parent df3ece6 commit 30e6a25
Show file tree
Hide file tree
Showing 7 changed files with 64 additions and 46 deletions.
15 changes: 11 additions & 4 deletions cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@ limitations under the License.
package main

import (
"context"
"crypto/tls"
"flag"
"os"
"os/signal"

// Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.)
// to ensure that exec-entrypoint and run can make use of them.
Expand All @@ -35,7 +37,7 @@ import (
metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
"sigs.k8s.io/controller-runtime/pkg/webhook"

infra1alpha1 "github.com/miscord-dev/cluster-api-provider-incus/api/v1alpha1"
infrav1alpha1 "github.com/miscord-dev/cluster-api-provider-incus/api/v1alpha1"
"github.com/miscord-dev/cluster-api-provider-incus/internal/controller"
// +kubebuilder:scaffold:imports
)
Expand All @@ -48,11 +50,14 @@ var (
func init() {
utilruntime.Must(clientgoscheme.AddToScheme(scheme))

utilruntime.Must(infra1alpha1.AddToScheme(scheme))
utilruntime.Must(infrav1alpha1.AddToScheme(scheme))
// +kubebuilder:scaffold:scheme
}

func main() {
ctx, cancel := signal.NotifyContext(context.Background(), os.Interrupt)
defer cancel()

var metricsAddr string
var enableLeaderElection bool
var probeAddr string
Expand Down Expand Up @@ -143,18 +148,20 @@ func main() {
setupLog.Error(err, "unable to start manager")
os.Exit(1)
}
logger := mgr.GetLogger()
ctx = ctrl.LoggerInto(ctx, logger)

if err = (&controller.IncusClusterReconciler{
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
}).SetupWithManager(mgr); err != nil {
}).SetupWithManager(ctx, mgr); err != nil {
setupLog.Error(err, "unable to create controller", "controller", "IncusCluster")
os.Exit(1)
}
if err = (&controller.IncusMachineReconciler{
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
}).SetupWithManager(mgr); err != nil {
}).SetupWithManager(ctx, mgr); err != nil {
setupLog.Error(err, "unable to create controller", "controller", "IncusMachine")
os.Exit(1)
}
Expand Down
28 changes: 19 additions & 9 deletions internal/controller/incuscluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,15 @@ import (
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
"sigs.k8s.io/cluster-api/util"
"sigs.k8s.io/cluster-api/util/patch"
"sigs.k8s.io/cluster-api/util/predicates"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/builder"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
"sigs.k8s.io/controller-runtime/pkg/handler"
"sigs.k8s.io/controller-runtime/pkg/log"

infra1alpha1 "github.com/miscord-dev/cluster-api-provider-incus/api/v1alpha1"
infrav1alpha1 "github.com/miscord-dev/cluster-api-provider-incus/api/v1alpha1"
)

// IncusClusterReconciler reconciles a IncusCluster object
Expand All @@ -57,7 +60,7 @@ func (r *IncusClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request
log := log.FromContext(ctx)

// Fetch the IncusCluster instance
incusCluster := &infra1alpha1.IncusCluster{}
incusCluster := &infrav1alpha1.IncusCluster{}
if err := r.Client.Get(ctx, req.NamespacedName, incusCluster); err != nil {
if apierrors.IsNotFound(err) {
return ctrl.Result{}, nil
Expand Down Expand Up @@ -98,22 +101,22 @@ func (r *IncusClusterReconciler) 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 !controllerutil.ContainsFinalizer(incusCluster, infra1alpha1.ClusterFinalizer) {
controllerutil.AddFinalizer(incusCluster, infra1alpha1.ClusterFinalizer)
if !controllerutil.ContainsFinalizer(incusCluster, infrav1alpha1.ClusterFinalizer) {
controllerutil.AddFinalizer(incusCluster, infrav1alpha1.ClusterFinalizer)
return ctrl.Result{}, nil
}

// Handle non-deleted clusters
return r.reconcileNormal(ctx, cluster, incusCluster)
}

func (r *IncusClusterReconciler) reconcileDelete(_ context.Context, cluster *clusterv1.Cluster, incusCluster *infra1alpha1.IncusCluster) error {
controllerutil.RemoveFinalizer(incusCluster, infra1alpha1.ClusterFinalizer)
func (r *IncusClusterReconciler) reconcileDelete(_ context.Context, cluster *clusterv1.Cluster, incusCluster *infrav1alpha1.IncusCluster) error {

Check failure on line 113 in internal/controller/incuscluster_controller.go

View workflow job for this annotation

GitHub Actions / Run on Ubuntu

`(*IncusClusterReconciler).reconcileDelete` - `cluster` is unused (unparam)
controllerutil.RemoveFinalizer(incusCluster, infrav1alpha1.ClusterFinalizer)

return nil
}

func (r *IncusClusterReconciler) reconcileNormal(ctx context.Context, cluster *clusterv1.Cluster, incusCluster *infra1alpha1.IncusCluster) (ctrl.Result, error) {
func (r *IncusClusterReconciler) reconcileNormal(ctx context.Context, cluster *clusterv1.Cluster, incusCluster *infrav1alpha1.IncusCluster) (ctrl.Result, error) {

Check failure on line 119 in internal/controller/incuscluster_controller.go

View workflow job for this annotation

GitHub Actions / Run on Ubuntu

`(*IncusClusterReconciler).reconcileNormal` - `cluster` is unused (unparam)
log := log.FromContext(ctx)

if incusCluster.Spec.ControlPlaneEndpoint.Host == "" {
Expand All @@ -134,9 +137,16 @@ func (r *IncusClusterReconciler) reconcileNormal(ctx context.Context, cluster *c
}

// SetupWithManager sets up the controller with the Manager.
func (r *IncusClusterReconciler) SetupWithManager(mgr ctrl.Manager) error {
func (r *IncusClusterReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager) error {
return ctrl.NewControllerManagedBy(mgr).
For(&infra1alpha1.IncusCluster{}).
For(&infrav1alpha1.IncusCluster{}).
Watches(
&clusterv1.Cluster{},
handler.EnqueueRequestsFromMapFunc(util.ClusterToInfrastructureMapFunc(ctx, infrav1alpha1.GroupVersion.WithKind("IncusCluster"), mgr.GetClient(), &infrav1alpha1.IncusCluster{})),
builder.WithPredicates(
predicates.ClusterUnpaused(ctrl.LoggerFrom(ctx)),
),
).
Named("incuscluster").
Complete(r)
}
8 changes: 4 additions & 4 deletions internal/controller/incuscluster_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import (

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

infra1alpha1 "github.com/miscord-dev/cluster-api-provider-incus/api/v1alpha1"
infrav1alpha1 "github.com/miscord-dev/cluster-api-provider-incus/api/v1alpha1"
)

var _ = Describe("IncusCluster Controller", func() {
Expand All @@ -40,13 +40,13 @@ var _ = Describe("IncusCluster Controller", func() {
Name: resourceName,
Namespace: "default", // TODO(user):Modify as needed
}
incuscluster := &infra1alpha1.IncusCluster{}
incuscluster := &infrav1alpha1.IncusCluster{}

BeforeEach(func() {
By("creating the custom resource for the Kind IncusCluster")
err := k8sClient.Get(ctx, typeNamespacedName, incuscluster)
if err != nil && errors.IsNotFound(err) {
resource := &infra1alpha1.IncusCluster{
resource := &infrav1alpha1.IncusCluster{
ObjectMeta: metav1.ObjectMeta{
Name: resourceName,
Namespace: "default",
Expand All @@ -59,7 +59,7 @@ var _ = Describe("IncusCluster Controller", func() {

AfterEach(func() {
// TODO(user): Cleanup logic after each test, like removing the resource instance.
resource := &infra1alpha1.IncusCluster{}
resource := &infrav1alpha1.IncusCluster{}
err := k8sClient.Get(ctx, typeNamespacedName, resource)
Expect(err).NotTo(HaveOccurred())

Expand Down
37 changes: 19 additions & 18 deletions internal/controller/incusmachine_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import (
"fmt"

"github.com/lxc/incus/shared/api"
infra1alpha1 "github.com/miscord-dev/cluster-api-provider-incus/api/v1alpha1"
infrav1alpha1 "github.com/miscord-dev/cluster-api-provider-incus/api/v1alpha1"
"github.com/miscord-dev/cluster-api-provider-incus/pkg/incus"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
Expand Down Expand Up @@ -68,7 +68,7 @@ func (r *IncusMachineReconciler) Reconcile(ctx context.Context, req ctrl.Request
ctx = ctrl.LoggerInto(ctx, log)

// Fetch the IncusMachine instance.
incusMachine := &infra1alpha1.IncusMachine{}
incusMachine := &infrav1alpha1.IncusMachine{}
if err := r.Client.Get(ctx, req.NamespacedName, incusMachine); err != nil {
if apierrors.IsNotFound(err) {
return ctrl.Result{}, nil
Expand Down Expand Up @@ -122,7 +122,7 @@ func (r *IncusMachineReconciler) Reconcile(ctx context.Context, req ctrl.Request
}

// Fetch the Incus Cluster.
incusCluster := &infra1alpha1.IncusCluster{}
incusCluster := &infrav1alpha1.IncusCluster{}
incusClusterName := client.ObjectKey{
Namespace: incusMachine.Namespace,
Name: cluster.Spec.InfrastructureRef.Name,
Expand All @@ -149,8 +149,8 @@ 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, infra1alpha1.MachineFinalizer) {
controllerutil.AddFinalizer(incusMachine, infra1alpha1.MachineFinalizer)
if incusMachine.ObjectMeta.DeletionTimestamp.IsZero() && !controllerutil.ContainsFinalizer(incusMachine, infrav1alpha1.MachineFinalizer) {
controllerutil.AddFinalizer(incusMachine, infrav1alpha1.MachineFinalizer)
return ctrl.Result{}, nil
}

Expand All @@ -163,13 +163,13 @@ func (r *IncusMachineReconciler) Reconcile(ctx context.Context, req ctrl.Request
return r.reconcileNormal(ctx, cluster, incusCluster, machine, incusMachine)
}

func patchIncusMachine(ctx context.Context, patchHelper *patch.Helper, incusMachine *infra1alpha1.IncusMachine) error {
func patchIncusMachine(ctx context.Context, patchHelper *patch.Helper, incusMachine *infrav1alpha1.IncusMachine) error {
// Always update the readyCondition by summarizing the state of other conditions.
// A step counter is added to represent progress during the provisioning process (instead we are hiding the step counter during the deletion process).
// conditions.SetSummary(incusMachine,
// conditions.WithConditions(
// infra1alpha1.ContainerProvisionedCondition,
// infra1alpha1.BootstrapExecSucceededCondition,
// infrav1alpha1.ContainerProvisionedCondition,
// infrav1alpha1.BootstrapExecSucceededCondition,
// ),
// conditions.WithStepCounterIf(incusMachine.ObjectMeta.DeletionTimestamp.IsZero() && incusMachine.Spec.ProviderID == nil),
// )
Expand All @@ -180,13 +180,13 @@ func patchIncusMachine(ctx context.Context, patchHelper *patch.Helper, incusMach
incusMachine,
// patch.WithOwnedConditions{Conditions: []clusterv1.ConditionType{
// clusterv1.ReadyCondition,
// infra1alpha1.ContainerProvisionedCondition,
// infra1alpha1.BootstrapExecSucceededCondition,
// infrav1alpha1.ContainerProvisionedCondition,
// infrav1alpha1.BootstrapExecSucceededCondition,
// }},
)
}

func (r *IncusMachineReconciler) reconcileDelete(ctx context.Context, incusCluster *infra1alpha1.IncusCluster, machine *clusterv1.Machine, incusMachine *infra1alpha1.IncusMachine) (ctrl.Result, error) {
func (r *IncusMachineReconciler) reconcileDelete(ctx context.Context, incusCluster *infrav1alpha1.IncusCluster, machine *clusterv1.Machine, incusMachine *infrav1alpha1.IncusMachine) (ctrl.Result, error) {

Check failure on line 189 in internal/controller/incusmachine_controller.go

View workflow job for this annotation

GitHub Actions / Run on Ubuntu

`(*IncusMachineReconciler).reconcileDelete` - `incusCluster` is unused (unparam)
log := ctrl.LoggerFrom(ctx)

// If the IncusMachine is being deleted, handle its deletion.
Expand All @@ -199,7 +199,7 @@ func (r *IncusMachineReconciler) reconcileDelete(ctx context.Context, incusClust
output, err := r.incusClient.GetInstance(ctx, incusMachine.Name)
if errors.Is(err, incus.ErrorInstanceNotFound) {
// Instance is already deleted so remove the finalizer.
controllerutil.RemoveFinalizer(incusMachine, infra1alpha1.MachineFinalizer)
controllerutil.RemoveFinalizer(incusMachine, infrav1alpha1.MachineFinalizer)
return ctrl.Result{}, nil
}
if err != nil {
Expand All @@ -222,7 +222,7 @@ func (r *IncusMachineReconciler) reconcileDelete(ctx context.Context, incusClust
}, nil
}

func (r *IncusMachineReconciler) reconcileNormal(ctx context.Context, cluster *clusterv1.Cluster, incusCluster *infra1alpha1.IncusCluster, machine *clusterv1.Machine, incusMachine *infra1alpha1.IncusMachine) (ctrl.Result, error) {
func (r *IncusMachineReconciler) reconcileNormal(ctx context.Context, cluster *clusterv1.Cluster, incusCluster *infrav1alpha1.IncusCluster, machine *clusterv1.Machine, incusMachine *infrav1alpha1.IncusMachine) (ctrl.Result, error) {

Check failure on line 225 in internal/controller/incusmachine_controller.go

View workflow job for this annotation

GitHub Actions / Run on Ubuntu

`(*IncusMachineReconciler).reconcileNormal` - `incusCluster` is unused (unparam)
log := ctrl.LoggerFrom(ctx)

// Check if the infrastructure is ready, otherwise return and wait for the cluster object to be updated
Expand Down Expand Up @@ -288,19 +288,19 @@ func (r *IncusMachineReconciler) getBootstrapData(ctx context.Context, namespace

// 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(), &infra1alpha1.IncusMachineList{}, mgr.GetScheme())
clusterToIncusMachines, err := util.ClusterToTypedObjectsMapper(mgr.GetClient(), &infrav1alpha1.IncusMachineList{}, mgr.GetScheme())
if err != nil {
return err
}

return ctrl.NewControllerManagedBy(mgr).
For(&infra1alpha1.IncusMachine{}).
For(&infrav1alpha1.IncusMachine{}).
Watches(
&clusterv1.Machine{},
handler.EnqueueRequestsFromMapFunc(util.MachineToInfrastructureMapFunc(infra1alpha1.GroupVersion.WithKind("IncusMachine"))),
handler.EnqueueRequestsFromMapFunc(util.MachineToInfrastructureMapFunc(infrav1alpha1.GroupVersion.WithKind("IncusMachine"))),
).
Watches(
&infra1alpha1.IncusCluster{},
&infrav1alpha1.IncusCluster{},
handler.EnqueueRequestsFromMapFunc(r.IncusClusterToIncusMachines),
).
Watches(
Expand All @@ -310,14 +310,15 @@ func (r *IncusMachineReconciler) SetupWithManager(ctx context.Context, mgr ctrl.
predicates.ClusterUnpausedAndInfrastructureReady(ctrl.LoggerFrom(ctx)),
),
).
Named("incusmachine").
Complete(r)
}

// IncusClusterToIncusMachines is a handler.ToRequestsFunc to be used to enqueue
// requests for reconciliation of IncusMachines.
func (r *IncusMachineReconciler) IncusClusterToIncusMachines(ctx context.Context, o client.Object) []ctrl.Request {
result := []ctrl.Request{}
c, ok := o.(*infra1alpha1.IncusCluster)
c, ok := o.(*infrav1alpha1.IncusCluster)
if !ok {
panic(fmt.Sprintf("Expected a IncusCluster but got a %T", o))
}
Expand Down
8 changes: 4 additions & 4 deletions internal/controller/incusmachine_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import (

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

infra1alpha1 "github.com/miscord-dev/cluster-api-provider-incus/api/v1alpha1"
infrav1alpha1 "github.com/miscord-dev/cluster-api-provider-incus/api/v1alpha1"
)

var _ = Describe("IncusMachine Controller", func() {
Expand All @@ -40,13 +40,13 @@ var _ = Describe("IncusMachine Controller", func() {
Name: resourceName,
Namespace: "default", // TODO(user):Modify as needed
}
incusmachine := &infra1alpha1.IncusMachine{}
incusmachine := &infrav1alpha1.IncusMachine{}

BeforeEach(func() {
By("creating the custom resource for the Kind IncusMachine")
err := k8sClient.Get(ctx, typeNamespacedName, incusmachine)
if err != nil && errors.IsNotFound(err) {
resource := &infra1alpha1.IncusMachine{
resource := &infrav1alpha1.IncusMachine{
ObjectMeta: metav1.ObjectMeta{
Name: resourceName,
Namespace: "default",
Expand All @@ -59,7 +59,7 @@ var _ = Describe("IncusMachine Controller", func() {

AfterEach(func() {
// TODO(user): Cleanup logic after each test, like removing the resource instance.
resource := &infra1alpha1.IncusMachine{}
resource := &infrav1alpha1.IncusMachine{}
err := k8sClient.Get(ctx, typeNamespacedName, resource)
Expect(err).NotTo(HaveOccurred())

Expand Down
4 changes: 2 additions & 2 deletions internal/controller/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import (
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/log/zap"

infra1alpha1 "github.com/miscord-dev/cluster-api-provider-incus/api/v1alpha1"
infrav1alpha1 "github.com/miscord-dev/cluster-api-provider-incus/api/v1alpha1"
// +kubebuilder:scaffold:imports
)

Expand Down Expand Up @@ -77,7 +77,7 @@ var _ = BeforeSuite(func() {
Expect(err).NotTo(HaveOccurred())
Expect(cfg).NotTo(BeNil())

err = infra1alpha1.AddToScheme(scheme.Scheme)
err = infrav1alpha1.AddToScheme(scheme.Scheme)
Expect(err).NotTo(HaveOccurred())

// +kubebuilder:scaffold:scheme
Expand Down
10 changes: 5 additions & 5 deletions pkg/incus/incus.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (

incusclient "github.com/lxc/incus/client"
"github.com/lxc/incus/shared/api"
infra1alpha1 "github.com/miscord-dev/cluster-api-provider-incus/api/v1alpha1"
infrav1alpha1 "github.com/miscord-dev/cluster-api-provider-incus/api/v1alpha1"
)

var (
Expand All @@ -24,12 +24,12 @@ type CreateInstanceInput struct {
Name string
BootstrapData BootstrapData

infra1alpha1.InstanceSpec
infrav1alpha1.InstanceSpec
}

type GetInstanceOutput struct {
Name string
infra1alpha1.InstanceSpec
infrav1alpha1.InstanceSpec

// TODO: Add status
StatusCode api.StatusCode
Expand Down Expand Up @@ -120,7 +120,7 @@ func (c *client) GetInstance(ctx context.Context, name string) (*GetInstanceOutp

return &GetInstanceOutput{
Name: name,
InstanceSpec: infra1alpha1.InstanceSpec{
InstanceSpec: infrav1alpha1.InstanceSpec{
Architecture: resp.Architecture,
Config: resp.Config,
Devices: resp.Devices,
Expand All @@ -129,7 +129,7 @@ func (c *client) GetInstance(ctx context.Context, name string) (*GetInstanceOutp
Restore: resp.Restore,
Stateful: resp.Stateful,
Description: resp.Description,
Type: infra1alpha1.InstanceType(resp.Type),
Type: infrav1alpha1.InstanceType(resp.Type),
},
StatusCode: resp.StatusCode,
}, nil
Expand Down

0 comments on commit 30e6a25

Please sign in to comment.