Skip to content

Commit

Permalink
pass instanceID instead of checking status
Browse files Browse the repository at this point in the history
  • Loading branch information
AshleyDumaine committed Aug 21, 2024
1 parent 68f0f1f commit 92ff04c
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 13 deletions.
6 changes: 1 addition & 5 deletions cloud/services/loadbalancers.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,17 +179,13 @@ func AddNodeToNB(
ctx context.Context,
logger logr.Logger,
machineScope *scope.MachineScope,
instanceID int,
) error {
// Update the NB backend with the new instance if it's a control plane node
if !kutil.IsControlPlaneMachine(machineScope.Machine) {
return nil
}

instanceID, err := util.GetInstanceID(machineScope.LinodeMachine.Spec.ProviderID)
if err != nil {
logger.Error(err, "Failed to parse instance ID from provider ID")
return err
}
// Get the private IP that was assigned
addresses, err := machineScope.LinodeClient.GetInstanceIPAddresses(ctx, instanceID)
if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions cloud/services/loadbalancers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -550,7 +550,7 @@ func TestAddNodeToNBConditions(t *testing.T) {
testcase.machineScope.Client = MockK8sClient
testcase.expectK8sClient(MockK8sClient)

err := AddNodeToNB(context.Background(), logr.Discard(), testcase.machineScope)
err := AddNodeToNB(context.Background(), logr.Discard(), testcase.machineScope, 123)
if testcase.expectedError != nil {
assert.ErrorContains(t, err, testcase.expectedError.Error())
}
Expand Down Expand Up @@ -724,7 +724,7 @@ func TestAddNodeToNBFullWorkflow(t *testing.T) {
testcase.machineScope.Client = MockK8sClient
testcase.expectK8sClient(MockK8sClient)

err := AddNodeToNB(context.Background(), logr.Discard(), testcase.machineScope)
err := AddNodeToNB(context.Background(), logr.Discard(), testcase.machineScope, 123)
if testcase.expectedError != nil {
assert.ErrorContains(t, err, testcase.expectedError.Error())
}
Expand Down
21 changes: 16 additions & 5 deletions controller/linodemachine_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ import (
crcontroller "sigs.k8s.io/controller-runtime/pkg/controller"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
"sigs.k8s.io/controller-runtime/pkg/handler"
"sigs.k8s.io/controller-runtime/pkg/reconcile"

infrav1alpha1 "github.com/linode/cluster-api-provider-linode/api/v1alpha1"
infrav1alpha2 "github.com/linode/cluster-api-provider-linode/api/v1alpha2"
Expand Down Expand Up @@ -140,10 +139,22 @@ func (r *LinodeMachineReconciler) Reconcile(ctx context.Context, req ctrl.Reques
cluster, err := kutil.GetClusterFromMetadata(ctx, r.TracedClient(), machine.ObjectMeta)
if err != nil {
log.Info("Failed to fetch cluster by label")
return reconcile.Result{}, nil
return ctrl.Result{}, nil

Check warning on line 142 in controller/linodemachine_controller.go

View check run for this annotation

Codecov / codecov/patch

controller/linodemachine_controller.go#L139-L142

Added lines #L139 - L142 were not covered by tests
}

// Fetch linode cluster
linodeClusterKey := client.ObjectKey{
Namespace: linodeMachine.Namespace,
Name: cluster.Spec.InfrastructureRef.Name,

Check warning on line 148 in controller/linodemachine_controller.go

View check run for this annotation

Codecov / codecov/patch

controller/linodemachine_controller.go#L146-L148

Added lines #L146 - L148 were not covered by tests
}
linodeCluster := &infrav1alpha2.LinodeCluster{}
if err := r.Client.Get(ctx, linodeClusterKey, linodeCluster); err != nil {
if err = client.IgnoreNotFound(err); err != nil {
return ctrl.Result{}, fmt.Errorf("get linodecluster %q: %w", linodeClusterKey, err)

Check warning on line 153 in controller/linodemachine_controller.go

View check run for this annotation

Codecov / codecov/patch

controller/linodemachine_controller.go#L150-L153

Added lines #L150 - L153 were not covered by tests
}
}

log = log.WithValues("cluster", cluster.Name)
log = log.WithValues("LinodeCluster", linodeCluster.Name)

Check warning on line 157 in controller/linodemachine_controller.go

View check run for this annotation

Codecov / codecov/patch

controller/linodemachine_controller.go#L157

Added line #L157 was not covered by tests

machineScope, err := scope.NewMachineScope(
ctx,
Expand All @@ -153,7 +164,7 @@ func (r *LinodeMachineReconciler) Reconcile(ctx context.Context, req ctrl.Reques
Client: r.TracedClient(),
Cluster: cluster,
Machine: machine,
LinodeCluster: &infrav1alpha2.LinodeCluster{},
LinodeCluster: linodeCluster,

Check warning on line 167 in controller/linodemachine_controller.go

View check run for this annotation

Codecov / codecov/patch

controller/linodemachine_controller.go#L167

Added line #L167 was not covered by tests
LinodeMachine: linodeMachine,
},
)
Expand Down Expand Up @@ -366,7 +377,7 @@ func (r *LinodeMachineReconciler) reconcileInstanceCreate(
}

if !reconciler.ConditionTrue(machineScope.LinodeMachine, ConditionPreflightNetworking) {
if err := r.addMachineToLB(ctx, machineScope); err != nil {
if err := r.addMachineToLB(ctx, machineScope, linodeInstance.ID); err != nil {
logger.Error(err, "Failed to add machine to LB")

if reconciler.RecordDecayingCondition(machineScope.LinodeMachine,
Expand Down
3 changes: 2 additions & 1 deletion controller/linodemachine_controller_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -481,10 +481,11 @@ func createInstanceConfigDeviceMap(instanceDisks map[string]*infrav1alpha2.Insta
func (r *LinodeMachineReconciler) addMachineToLB(
ctx context.Context,
machineScope *scope.MachineScope,
linodeInstanceID int,
) error {
logger := logr.FromContextOrDiscard(ctx)
if machineScope.LinodeCluster.Spec.Network.LoadBalancerType != "dns" {
if err := services.AddNodeToNB(ctx, logger, machineScope); err != nil {
if err := services.AddNodeToNB(ctx, logger, machineScope, linodeInstanceID); err != nil {
return err

Check warning on line 489 in controller/linodemachine_controller_helpers.go

View check run for this annotation

Codecov / codecov/patch

controller/linodemachine_controller_helpers.go#L489

Added line #L489 was not covered by tests
}
} else {
Expand Down

0 comments on commit 92ff04c

Please sign in to comment.