Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion internal/controller/nodeclaim_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,9 @@ func (f *fakeProvider) Terminate(_ context.Context, id, region string) error {
f.regions = append(f.regions, region)
return f.terminateErr
}
func (f *fakeProvider) Get(context.Context, string) (*provider.Instance, error) { return nil, nil }
func (f *fakeProvider) Get(context.Context, string, string) (*provider.Instance, error) {
return nil, nil
}
func (f *fakeProvider) List(context.Context) ([]provider.Instance, error) {
return f.list, f.listErr
}
Expand Down
30 changes: 24 additions & 6 deletions pkg/provider/aws/aws.go
Original file line number Diff line number Diff line change
Expand Up @@ -545,12 +545,30 @@ func (p *Provider) Terminate(ctx context.Context, instanceID, region string) err
return lastErr
}

// Get implements provider.Provider. instanceID is a raw EC2 id, which does not
// carry its region, so the lookup sweeps the swept regions and returns the first
// region's view of the instance. A per-region client-build/describe error is
// tolerated and the sweep continues; only if every region errored (and none held
// the instance) is that error surfaced.
func (p *Provider) Get(ctx context.Context, instanceID string) (*provider.Instance, error) {
// Get implements provider.Provider. As in Terminate, the region names the endpoint that
// can answer for the instance, and Describe there is the whole call.
//
// Without a region the lookup sweeps and returns the first region's view. A per-region
// client-build/describe error is tolerated and the sweep continues; only if every region
// errored (and none held the instance) is that error surfaced. An unswept region reads as
// (nil, nil) — indistinguishable from terminated, which is why the region is worth passing.
func (p *Provider) Get(ctx context.Context, instanceID, region string) (*provider.Instance, error) {
if region != "" {
client, err := p.clientFor(ctx, region)
if err != nil {
return nil, err
}
ec2, err := client.DescribeInstance(ctx, instanceID)
if err != nil {
return nil, err
}
if ec2 == nil {
return nil, nil // gone from the region that owns it: terminated
}
inst := p.toInstance(*ec2)
return &inst, nil
}

var lastErr error
for _, region := range p.sweepRegions() {
client, err := p.clientFor(ctx, region)
Expand Down
10 changes: 5 additions & 5 deletions pkg/provider/aws/aws_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -415,8 +415,8 @@ func TestGetAndList_NormalizeInstance(t *testing.T) {
}
p := newTestProvider(f)

// Get takes the raw EC2 id Provision/List hand back; it sweeps regions to find it.
got, err := p.Get(context.Background(), "i-1")
// Get with no region falls back to sweeping for the instance.
got, err := p.Get(context.Background(), "i-1", "")
if err != nil {
t.Fatalf("Get: %v", err)
}
Expand All @@ -437,7 +437,7 @@ func TestGetAndList_NormalizeInstance(t *testing.T) {
}

// A missing instance is (nil, nil): absence == terminated per the contract.
missing, err := p.Get(context.Background(), "i-gone")
missing, err := p.Get(context.Background(), "i-gone", testRegion)
if err != nil {
t.Fatalf("Get(missing): %v", err)
}
Expand All @@ -449,8 +449,8 @@ func TestGetAndList_NormalizeInstance(t *testing.T) {
if err != nil {
t.Fatalf("List: %v", err)
}
// List reports the raw EC2 id; a downstream Terminate re-locates it by sweeping
// regions.
// List reports the raw EC2 id, alongside the Region a downstream Get/Terminate should
// pass back so neither has to search for it.
if len(list) != 1 || list[0].ID != "i-1" {
t.Fatalf("List = %+v", list)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/provider/fake/fake.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ func (p *Provider) Terminate(_ context.Context, instanceID, _ string) error {
}

// Get returns one instance, or (nil, nil) if it no longer exists.
func (p *Provider) Get(_ context.Context, instanceID string) (*provider.Instance, error) {
func (p *Provider) Get(_ context.Context, instanceID, _ string) (*provider.Instance, error) {
p.mu.Lock()
defer p.mu.Unlock()
inst, ok := p.instances[instanceID]
Expand Down
4 changes: 2 additions & 2 deletions pkg/provider/fake/fake_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func TestProvisionReportsRunningAndLists(t *testing.T) {
}

// Get reports it Running with the claim recovered.
inst, err := p.Get(ctx, id)
inst, err := p.Get(ctx, id, "")
if err != nil || inst == nil {
t.Fatalf("Get = (%v, %v), want a live instance", inst, err)
}
Expand Down Expand Up @@ -120,7 +120,7 @@ func TestTerminateIsIdempotent(t *testing.T) {
t.Fatalf("Terminate: %v", err)
}
// Gone from Get/List.
if inst, _ := p.Get(ctx, id); inst != nil {
if inst, _ := p.Get(ctx, id, ""); inst != nil {
t.Fatalf("Get after Terminate = %v, want nil (terminated)", inst)
}
// A repeat Terminate (and terminating an unknown id) is a no-op, not an error.
Expand Down
4 changes: 2 additions & 2 deletions pkg/provider/modal/modal.go
Original file line number Diff line number Diff line change
Expand Up @@ -406,8 +406,8 @@ func (p *Provider) Exec(
return p.client.SandboxExec(ctx, instanceID, cmd, opts)
}

// Get implements provider.Provider.
func (p *Provider) Get(ctx context.Context, instanceID string) (*provider.Instance, error) {
// Get implements provider.Provider. The region is ignored, as in Terminate.
func (p *Provider) Get(ctx context.Context, instanceID, _ string) (*provider.Instance, error) {
sb, err := p.client.GetSandbox(ctx, instanceID)
if err != nil {
return nil, err
Expand Down
5 changes: 4 additions & 1 deletion pkg/provider/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,10 @@ type Provider interface {

// Get returns the current state of one instance, or (nil, nil) if it no
// longer exists (treat absence as terminated).
Get(ctx context.Context, instanceID string) (*Instance, error)
//
// region is as in Terminate, and matters more here: absence is read as terminated, so a
// provider that had to search and searched the wrong place reports a live instance dead.
Get(ctx context.Context, instanceID, region string) (*Instance, error)

// List returns every instance Nebula owns on this provider, in as few API calls as
// possible (ideally one). This is the engine of the poll loop: since no provider pushes
Expand Down
4 changes: 3 additions & 1 deletion pkg/vnode/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,9 @@ func (f *fakeProvider) Terminate(_ context.Context, id, region string) error {
return f.terminateErr
}

func (f *fakeProvider) Get(context.Context, string) (*provider.Instance, error) { return nil, nil }
func (f *fakeProvider) Get(context.Context, string, string) (*provider.Instance, error) {
return nil, nil
}
func (f *fakeProvider) List(context.Context) ([]provider.Instance, error) {
return f.list, f.listErr
}
Expand Down
Loading