From e43c567cd8249b967a3fd83a8a7fc61227f76f43 Mon Sep 17 00:00:00 2001 From: Huw Jones Date: Thu, 24 Oct 2024 12:10:58 +0100 Subject: [PATCH] cmd/assignAddress: return assignedAddress --- cmd/main.go | 21 +++++++++++---------- cmd/main_test.go | 26 ++++++++++++++++---------- 2 files changed, 27 insertions(+), 20 deletions(-) diff --git a/cmd/main.go b/cmd/main.go index c37a934..bd49d13 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -82,7 +82,7 @@ func prepareLogger(level string, json bool) *logrus.Entry { return log } -func assignAddress(c context.Context, log *logrus.Entry, client kubernetes.Interface, assigner address.Assigner, node *types.Node, cfg *config.Config) error { +func assignAddress(c context.Context, log *logrus.Entry, client kubernetes.Interface, assigner address.Assigner, node *types.Node, cfg *config.Config) (string, error) { ctx, cancel := context.WithCancel(c) defer cancel() @@ -101,22 +101,23 @@ func assignAddress(c context.Context, log *logrus.Entry, client kubernetes.Inter "retry-counter": retryCounter, "retry-attempts": cfg.RetryAttempts, }).Debug("assigning static public IP address to node") - err := func(ctx context.Context) error { + assignedAddress, err := func(ctx context.Context) (string, error) { if err := lock.Lock(ctx); err != nil { - return errors.Wrap(err, "failed to acquire lock") + return "", errors.Wrap(err, "failed to acquire lock") } log.Debug("lock acquired") defer func() { lock.Unlock(ctx) //nolint:errcheck log.Debug("lock released") }() - if _, err := assigner.Assign(ctx, node.Instance, node.Zone, cfg.Filter, cfg.OrderBy); err != nil { - return err //nolint:wrapcheck + assignedAddress, err := assigner.Assign(ctx, node.Instance, node.Zone, cfg.Filter, cfg.OrderBy) + if err != nil { + return "", err //nolint:wrapcheck } - return nil + return assignedAddress, nil }(c) if err == nil || errors.Is(err, address.ErrStaticIPAlreadyAssigned) { - return nil + return assignedAddress, nil } log.WithError(err).WithFields(logrus.Fields{ @@ -130,10 +131,10 @@ func assignAddress(c context.Context, log *logrus.Entry, client kubernetes.Inter continue case <-ctx.Done(): // If the context is done, return an error indicating that the operation was cancelled - return errors.Wrap(ctx.Err(), "context cancelled while assigning addresses") + return "", errors.Wrap(ctx.Err(), "context cancelled while assigning addresses") } } - return errors.New("reached maximum number of retries") + return "", errors.New("reached maximum number of retries") } func run(c context.Context, log *logrus.Entry, cfg *config.Config) error { @@ -169,7 +170,7 @@ func run(c context.Context, log *logrus.Entry, cfg *config.Config) error { return errors.Wrap(err, "initializing assigner") } - err = assignAddress(ctx, log, clientset, assigner, n, cfg) + _, err = assignAddress(ctx, log, clientset, assigner, n, cfg) if err != nil { return errors.Wrap(err, "assigning static public IP address") } diff --git a/cmd/main_test.go b/cmd/main_test.go index 68439eb..57982f1 100644 --- a/cmd/main_test.go +++ b/cmd/main_test.go @@ -24,15 +24,17 @@ func Test_assignAddress(t *testing.T) { tests := []struct { name string args args + address string wantErr bool }{ { - name: "assign address successfully", + name: "assign address successfully", + address: "1.1.1.1", args: args{ c: context.Background(), assignerFn: func(t *testing.T) address.Assigner { mock := mocks.NewAssigner(t) - mock.EXPECT().Assign(tmock.Anything, "test-instance", "test-zone", []string{"test-filter"}, "test-order-by").Return(nil) + mock.EXPECT().Assign(tmock.Anything, "test-instance", "test-zone", []string{"test-filter"}, "test-order-by").Return("1.1.1.1", nil) return mock }, node: &types.Node{ @@ -51,14 +53,15 @@ func Test_assignAddress(t *testing.T) { }, }, { - name: "assign address after a few retries", + name: "assign address after a few retries", + address: "1.1.1.1", args: args{ c: context.Background(), assignerFn: func(t *testing.T) address.Assigner { mock := mocks.NewAssigner(t) - mock.EXPECT().Assign(tmock.Anything, "test-instance", "test-zone", []string{"test-filter"}, "test-order-by").Return(errors.New("first error")).Once() - mock.EXPECT().Assign(tmock.Anything, "test-instance", "test-zone", []string{"test-filter"}, "test-order-by").Return(errors.New("second error")).Once() - mock.EXPECT().Assign(tmock.Anything, "test-instance", "test-zone", []string{"test-filter"}, "test-order-by").Return(nil).Once() + mock.EXPECT().Assign(tmock.Anything, "test-instance", "test-zone", []string{"test-filter"}, "test-order-by").Return("", errors.New("first error")).Once() + mock.EXPECT().Assign(tmock.Anything, "test-instance", "test-zone", []string{"test-filter"}, "test-order-by").Return("", errors.New("second error")).Once() + mock.EXPECT().Assign(tmock.Anything, "test-instance", "test-zone", []string{"test-filter"}, "test-order-by").Return("1.1.1.1", nil).Once() return mock }, node: &types.Node{ @@ -82,7 +85,7 @@ func Test_assignAddress(t *testing.T) { c: context.Background(), assignerFn: func(t *testing.T) address.Assigner { mock := mocks.NewAssigner(t) - mock.EXPECT().Assign(tmock.Anything, "test-instance", "test-zone", []string{"test-filter"}, "test-order-by").Return(errors.New("error")).Times(4) + mock.EXPECT().Assign(tmock.Anything, "test-instance", "test-zone", []string{"test-filter"}, "test-order-by").Return("", errors.New("error")).Times(4) return mock }, node: &types.Node{ @@ -115,7 +118,7 @@ func Test_assignAddress(t *testing.T) { }(), assignerFn: func(t *testing.T) address.Assigner { mock := mocks.NewAssigner(t) - mock.EXPECT().Assign(tmock.Anything, "test-instance", "test-zone", []string{"test-filter"}, "test-order-by").Return(errors.New("error")).Maybe() + mock.EXPECT().Assign(tmock.Anything, "test-instance", "test-zone", []string{"test-filter"}, "test-order-by").Return("", errors.New("error")).Maybe() return mock }, node: &types.Node{ @@ -143,7 +146,7 @@ func Test_assignAddress(t *testing.T) { }(), assignerFn: func(t *testing.T) address.Assigner { mock := mocks.NewAssigner(t) - mock.EXPECT().Assign(tmock.Anything, "test-instance", "test-zone", []string{"test-filter"}, "test-order-by").Return(errors.New("error")).Maybe() + mock.EXPECT().Assign(tmock.Anything, "test-instance", "test-zone", []string{"test-filter"}, "test-order-by").Return("", errors.New("error")).Maybe() return mock }, node: &types.Node{ @@ -168,8 +171,11 @@ func Test_assignAddress(t *testing.T) { log := prepareLogger("debug", false) assigner := tt.args.assignerFn(t) client := fake.NewSimpleClientset() - if err := assignAddress(tt.args.c, log, client, assigner, tt.args.node, tt.args.cfg); (err != nil) != tt.wantErr { + assignedAddress, err := assignAddress(tt.args.c, log, client, assigner, tt.args.node, tt.args.cfg) + if err != nil != tt.wantErr { t.Errorf("assignAddress() error = %v, wantErr %v", err, tt.wantErr) + } else if assignedAddress != tt.address { + t.Fatalf("assignAddress() = %v, want %v", assignedAddress, tt.address) } }) }