diff --git a/.golangci.yaml b/.golangci.yaml index be1933f..e50b1b8 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -6,6 +6,7 @@ linters: enable: - godoclint - unparam + - goheader disable: - errcheck settings: @@ -16,6 +17,11 @@ linters: options: max-len: length: 100 + goheader: + template: |- + This Source Code Form is subject to the terms of the Mozilla Public + License, v. 2.0. If a copy of the MPL was not distributed with this + file, You can obtain one at https://mozilla.org/MPL/2.0/. formatters: enable: diff --git a/Makefile b/Makefile index 9f51719..c24d801 100644 --- a/Makefile +++ b/Makefile @@ -96,8 +96,12 @@ helm-push: helm-package fmt: @ echo "+ Formatting Go code..." @ go tool -modfile tools/go.mod golangci-lint fmt + @ go tool -modfile tools/go.mod golangci-lint run --enable-only=goheader --fix .PHONY: lint lint: @ echo "+ Running Go linters..." @ go tool -modfile tools/go.mod golangci-lint run + +.PHONY: dev +dev: fmt lint test diff --git a/internal/provider/instances_v2.go b/internal/provider/instances_v2.go index 1824c28..ab00f09 100644 --- a/internal/provider/instances_v2.go +++ b/internal/provider/instances_v2.go @@ -12,7 +12,6 @@ import ( "github.com/oxidecomputer/oxide.go/oxide" v1 "k8s.io/api/core/v1" - "k8s.io/client-go/kubernetes" cloudprovider "k8s.io/cloud-provider" ) @@ -21,41 +20,46 @@ var _ cloudprovider.InstancesV2 = (*InstancesV2)(nil) // gibibyte is the number of bytes in a gibibyte. const gibibyte = 1024 * 1024 * 1024 +type oxideInstanceClient interface { + InstanceNetworkInterfaceList( + context.Context, + oxide.InstanceNetworkInterfaceListParams, + ) (*oxide.InstanceNetworkInterfaceResultsPage, error) + InstanceExternalIpList( + context.Context, + oxide.InstanceExternalIpListParams, + ) (*oxide.ExternalIpResultsPage, error) + InstanceView(context.Context, oxide.InstanceViewParams) (*oxide.Instance, error) +} + // InstancesV2 implements [cloudprovider.InstancesV2] to provide Oxide specific // instance functionality. type InstancesV2 struct { - client *oxide.Client + client oxideInstanceClient project string - - k8sClient kubernetes.Interface } -// InstanceExists checks whether the provided Kubernetes node exists as instance -// in Oxide. +// InstanceExists checks whether the provided Kubernetes node exists as an instance +// in Oxide. The cloud node lifecycle controller uses this information to determine +// if it can delete the Node object. func (i *InstancesV2) InstanceExists(ctx context.Context, node *v1.Node) (bool, error) { ctx, cancel := context.WithTimeout(ctx, 30*time.Second) defer cancel() - instanceID, err := InstanceIDFromProviderID(node.Spec.ProviderID) + // Get the instance, either from the provider ID or by looking up by name. + _, err := i.getInstance(ctx, node) if err != nil { - return false, fmt.Errorf("failed retrieving instance id from provider id: %w", err) - } - - if _, err := i.client.InstanceView(ctx, oxide.InstanceViewParams{ - Instance: oxide.NameOrId(instanceID), - }); err != nil { if errors.Is(err, oxide.ErrObjectNotFound) { return false, nil } - - return false, fmt.Errorf("failed viewing oxide instance %s: %w", instanceID, err) + return false, err } - return true, nil } -// InstanceMetadata populates the metadata for the provided node, notably -// setting its provider ID. +// InstanceMetadata is called by the cloud node controller to initialize nodes with +// the node.cloudprovider.kubernetes.io/uninitialized:NoSchedule taint. It returns +// metadata for the provided node, notably its provider ID. func (i *InstancesV2) InstanceMetadata( ctx context.Context, node *v1.Node, @@ -146,6 +150,25 @@ func (i *InstancesV2) InstanceMetadata( }, nil } +// InstanceShutdown checks whether the provided node is shut down in Oxide. +// The cloud node lifecycle controller uses this to determine if the +// node.cloudprovider.kubernetes.io/shutdown:NoSchedule taint should +// be applied to the Node object. +func (i *InstancesV2) InstanceShutdown(ctx context.Context, node *v1.Node) (bool, error) { + ctx, cancel := context.WithTimeout(ctx, 30*time.Second) + defer cancel() + + // Get the instance, either from the provider ID or by looking up by name. + instance, err := i.getInstance(ctx, node) + if err != nil { + if errors.Is(err, oxide.ErrObjectNotFound) { + return true, nil + } + return false, err + } + return instance.RunState == oxide.InstanceStateStopped, nil +} + // getInstance retrieves the instance either from the node's provider ID // or by looking up the instance by name. func (i *InstancesV2) getInstance(ctx context.Context, node *v1.Node) (*oxide.Instance, error) { @@ -170,23 +193,3 @@ func (i *InstancesV2) getInstance(ctx context.Context, node *v1.Node) (*oxide.In return instance, nil } - -// InstanceShutdown checks whether the provided node is shut down in Oxide. -func (i *InstancesV2) InstanceShutdown(ctx context.Context, node *v1.Node) (bool, error) { - ctx, cancel := context.WithTimeout(ctx, 30*time.Second) - defer cancel() - - instanceID, err := InstanceIDFromProviderID(node.Spec.ProviderID) - if err != nil { - return false, fmt.Errorf("failed retrieving instance id from provider id: %w", err) - } - - instance, err := i.client.InstanceView(ctx, oxide.InstanceViewParams{ - Instance: oxide.NameOrId(instanceID), - }) - if err != nil { - return false, fmt.Errorf("failed viewing oxide instance %s: %w", instanceID, err) - } - - return instance.RunState == oxide.InstanceStateStopped, nil -} diff --git a/internal/provider/instances_v2_test.go b/internal/provider/instances_v2_test.go new file mode 100644 index 0000000..40badcf --- /dev/null +++ b/internal/provider/instances_v2_test.go @@ -0,0 +1,202 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +package provider + +import ( + "context" + "testing" + + "github.com/oxidecomputer/oxide.go/oxide" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +type mockOxideClient struct { + InstanceNetworkInterfaceListOutput *oxide.InstanceNetworkInterfaceResultsPage + InstanceNetworkInterfaceListError error + + InstanceExternalIpListOutput *oxide.ExternalIpResultsPage + InstanceExternalIpListError error + + InstanceViewOutput *oxide.Instance + InstanceViewError error +} + +var ( + nodeWithoutProviderID = v1.Node{ + ObjectMeta: metav1.ObjectMeta{Name: "node-1"}, + Spec: v1.NodeSpec{}, + } + nodeWithProviderID = v1.Node{ + ObjectMeta: metav1.ObjectMeta{Name: "node-1"}, + Spec: v1.NodeSpec{ + ProviderID: "oxide://12345678-1234-1234-1234-123456789abc", + }, + } + nodeDoesNotExistInOxide = v1.Node{ + ObjectMeta: metav1.ObjectMeta{Name: "node-2"}, + Spec: v1.NodeSpec{ + ProviderID: "oxide://87654321-1234-1234-1234-123456789abc", + }, + } + + instanceRunning = oxide.Instance{ + Name: oxide.Name("node-1"), + Id: "12345678-1234-1234-1234-123456789abc", + RunState: oxide.InstanceStateRunning, + } + + instanceStopped = oxide.Instance{ + Name: oxide.Name("node-1"), + Id: "12345678-1234-1234-1234-123456789abc", + RunState: oxide.InstanceStateStopped, + } +) + +func TestInstanceExists(t *testing.T) { + t.Run("WithProviderID", func(t *testing.T) { + instancesV2 := InstancesV2{ + client: &mockOxideClient{ + InstanceViewOutput: &instanceRunning, + }, + project: "test", + } + exists, err := instancesV2.InstanceExists(t.Context(), &nodeWithProviderID) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !exists { + t.Fatal("expected instance to exist via the Oxide API") + } + }) + + t.Run("NoProviderID", func(t *testing.T) { + instancesV2 := InstancesV2{ + client: &mockOxideClient{ + InstanceViewOutput: &instanceRunning, + }, + project: "test", + } + exists, err := instancesV2.InstanceExists(t.Context(), &nodeWithoutProviderID) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !exists { + t.Fatal("expected instance to exist via the Oxide API") + } + }) + + t.Run("DoesNotExistInOxide", func(t *testing.T) { + instancesV2 := InstancesV2{ + client: &mockOxideClient{ + InstanceViewError: oxide.ErrObjectNotFound, + }, + project: "test", + } + exists, err := instancesV2.InstanceExists(t.Context(), &nodeDoesNotExistInOxide) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if exists { + t.Fatal("expected instance to NOT exist via the Oxide API") + } + }) +} + +func TestShutdown(t *testing.T) { + t.Run("RunningWithProviderID", func(t *testing.T) { + instancesV2 := InstancesV2{ + client: &mockOxideClient{ + InstanceViewOutput: &instanceRunning, + }, + project: "test", + } + shutdown, err := instancesV2.InstanceShutdown(t.Context(), &nodeWithProviderID) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if shutdown { + t.Fatal("expected instance to be running via the Oxide API") + } + }) + + t.Run("RunningNoProviderID", func(t *testing.T) { + instancesV2 := InstancesV2{ + client: &mockOxideClient{ + InstanceViewOutput: &instanceRunning, + }, + project: "test", + } + shutdown, err := instancesV2.InstanceShutdown(t.Context(), &nodeWithoutProviderID) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if shutdown { + t.Fatal("expected instance to be running via the Oxide API") + } + }) + + t.Run("InstanceStopped", func(t *testing.T) { + instancesV2 := InstancesV2{ + client: &mockOxideClient{ + InstanceViewOutput: &instanceStopped, + }, + project: "test", + } + shutdown, err := instancesV2.InstanceShutdown(t.Context(), &nodeDoesNotExistInOxide) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !shutdown { + t.Fatal("expected instance to be stopped via the Oxide API") + } + }) + + t.Run("DoesNotExistInOxide", func(t *testing.T) { + instancesV2 := InstancesV2{ + client: &mockOxideClient{ + InstanceViewError: oxide.ErrObjectNotFound, + }, + project: "test", + } + shutdown, err := instancesV2.InstanceShutdown(t.Context(), &nodeDoesNotExistInOxide) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !shutdown { + t.Fatal("expected instance to NOT exist via the Oxide API") + } + }) +} + +func (c *mockOxideClient) InstanceNetworkInterfaceList( + context.Context, + oxide.InstanceNetworkInterfaceListParams, +) (*oxide.InstanceNetworkInterfaceResultsPage, error) { + if c.InstanceNetworkInterfaceListError != nil { + return nil, c.InstanceNetworkInterfaceListError + } + return c.InstanceNetworkInterfaceListOutput, nil +} + +func (c *mockOxideClient) InstanceExternalIpList( + context.Context, + oxide.InstanceExternalIpListParams, +) (*oxide.ExternalIpResultsPage, error) { + if c.InstanceExternalIpListError != nil { + return nil, c.InstanceExternalIpListError + } + return c.InstanceExternalIpListOutput, nil +} + +func (c *mockOxideClient) InstanceView( + context.Context, + oxide.InstanceViewParams, +) (*oxide.Instance, error) { + if c.InstanceViewError != nil { + return nil, c.InstanceViewError + } + return c.InstanceViewOutput, nil +} diff --git a/internal/provider/provider.go b/internal/provider/provider.go index a2c9b30..bf9db2c 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -104,9 +104,8 @@ func (o *Oxide) Instances() (cloudprovider.Instances, bool) { // metadata, and determine whether they exists to facilitate cleanup. func (o *Oxide) InstancesV2() (cloudprovider.InstancesV2, bool) { return &InstancesV2{ - client: o.client, - project: o.project, - k8sClient: o.k8sClient, + client: o.client, + project: o.project, }, true }