diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 735a615..8e2c3f0 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -61,7 +61,13 @@ jobs: runs-on: ubuntu-latest needs: validate # build only on master branch and tags - if: ${{ !contains(github.event.head_commit.message,'[skip ci]') && (github.event_name == 'push' && (github.ref == 'refs/heads/master' || startsWith(github.ref, 'refs/tags/'))) }} + if: ${{ + !contains(github.event.head_commit.message, '[skip ci]') && + ( + (github.event_name == 'push' && (github.ref == 'refs/heads/master' || startsWith(github.ref, 'refs/tags/'))) || + (github.event_name == 'pull_request' && github.event.pull_request.draft == false) + ) + }} steps: - name: checkout uses: actions/checkout@v4 @@ -82,7 +88,6 @@ jobs: uses: docker/setup-buildx-action@v3 - name: login to DockerHub - if: ${{ github.event_name != 'pull_request' }} uses: docker/login-action@v3 with: username: ${{ secrets.DOCKERHUB_USERNAME }} diff --git a/cmd/main.go b/cmd/main.go index 79ae0a3..17080ac 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -89,6 +89,10 @@ func assignAddress(c context.Context, log *logrus.Entry, assigner address.Assign for { err := assigner.Assign(ctx, node.Instance, node.Zone, cfg.Filter, cfg.OrderBy) + if err != nil && errors.Is(err, address.ErrStaticIPAlreadyAssigned) { + log.Infof("static public IP address already assigned to node instance %s", node.Instance) + return nil + } if err != nil { log.WithError(err).Errorf("failed to assign static public IP address to node %s", node.Name) if retryCounter < cfg.RetryAttempts { @@ -105,9 +109,8 @@ func assignAddress(c context.Context, log *logrus.Entry, assigner address.Assign return errors.Wrap(err, "context is done") } } - break + return nil } - return nil } func run(c context.Context, log *logrus.Entry, cfg *config.Config) error { diff --git a/examples/aws/eks.tf b/examples/aws/eks.tf index a784f7d..63db025 100644 --- a/examples/aws/eks.tf +++ b/examples/aws/eks.tf @@ -222,7 +222,7 @@ resource "kubernetes_daemonset" "kubeip_daemonset" { priority_class_name = "system-node-critical" container { name = "kubeip-agent" - image = "doitintl/kubeip-agent" + image = "doitintl/kubeip-agent:${var.kubeip_version}" env { name = "NODE_NAME" value_from { diff --git a/examples/aws/variables.tf b/examples/aws/variables.tf index d8a590b..ae876b0 100644 --- a/examples/aws/variables.tf +++ b/examples/aws/variables.tf @@ -36,4 +36,9 @@ variable "public_cidr_ranges" { variable "kubernetes_version" { type = string default = "1.28" +} + +variable "kubeip_version" { + type = string + default = "latest" } \ No newline at end of file diff --git a/examples/gcp/gke.tf b/examples/gcp/gke.tf index f63a0b6..fbb9243 100644 --- a/examples/gcp/gke.tf +++ b/examples/gcp/gke.tf @@ -281,7 +281,7 @@ resource "kubernetes_daemonset" "kubeip_daemonset" { priority_class_name = "system-node-critical" container { name = "kubeip-agent" - image = "doitintl/kubeip-agent" + image = "doitintl/kubeip-agent:${var.kubeip_version}" env { name = "NODE_NAME" value_from { diff --git a/examples/gcp/variables.tf b/examples/gcp/variables.tf index 9f12bce..ef662e5 100644 --- a/examples/gcp/variables.tf +++ b/examples/gcp/variables.tf @@ -50,4 +50,9 @@ variable "machine_type" { variable "ipv6_support" { type = bool default = false +} + +variable "kubeip_version" { + type = string + default = "latest" } \ No newline at end of file diff --git a/internal/address/assigner.go b/internal/address/assigner.go index 394fe93..d94cfb7 100644 --- a/internal/address/assigner.go +++ b/internal/address/assigner.go @@ -10,7 +10,9 @@ import ( ) var ( - ErrUnknownCloudProvider = errors.New("unknown cloud provider") + ErrUnknownCloudProvider = errors.New("unknown cloud provider") + ErrStaticIPAlreadyAssigned = errors.New("static public IP already assigned") + ErrNoStaticIPAssigned = errors.New("no static public IP assigned") ) type Assigner interface { diff --git a/internal/address/aws.go b/internal/address/aws.go index f7e891b..910abda 100644 --- a/internal/address/aws.go +++ b/internal/address/aws.go @@ -165,54 +165,77 @@ func (a *awsAssigner) forceCheckAddressAssigned(ctx context.Context, allocationI return false, nil } -//nolint:funlen,gocyclo func (a *awsAssigner) Assign(ctx context.Context, instanceID, _ string, filter []string, orderBy string) error { // get elastic IP attached to the instance - filters := make(map[string][]string) - filters["instance-id"] = []string{instanceID} - addresses, err := a.eipLister.List(ctx, filters, true) + err := a.checkElasticIPAssigned(ctx, instanceID) if err != nil { - return errors.Wrapf(err, "failed to list elastic IPs attached to instance %s", instanceID) + return errors.Wrapf(err, "check if elastic IP is already assigned to instance %s", instanceID) } - if len(addresses) > 0 { - a.logger.Infof("elastic IP %s is already attached to instance %s", *addresses[0].PublicIp, instanceID) - return nil + + // get available elastic IPs based on filter and orderBy + addresses, err := a.getAvailableElasticIPs(ctx, filter, orderBy) + if err != nil { + return errors.Wrap(err, "failed to get available elastic IPs") } - // get available elastic IPs - filters = make(map[string][]string) - for _, f := range filter { - name, values, err2 := parseShorthandFilter(f) - if err2 != nil { - return errors.Wrapf(err2, "failed to parse filter %s", f) - } - filters[name] = values + // get EC2 instance + instance, err := a.instanceGetter.Get(ctx, instanceID, a.region) + if err != nil { + return errors.Wrapf(err, "failed to get instance %s", instanceID) } - addresses, err = a.eipLister.List(context.Background(), filters, false) + // get primary network interface ID with public IP address (DeviceIndex == 0) + networkInterfaceID, err := a.getNetworkInterfaceID(instance) if err != nil { - return errors.Wrap(err, "failed to list available elastic IPs") + return errors.Wrapf(err, "failed to get network interface ID for instance %s", instanceID) } - // if no available elastic IPs, return error - if len(addresses) == 0 { - return errors.Errorf("no available elastic IPs") + // try to assign available addresses until succeeds + // due to concurrency, it is possible that another kubeip instance will assign the same address + for i := range addresses { + a.logger.WithFields(logrus.Fields{ + "instance": instanceID, + "address": *addresses[i].PublicIp, + "allocation_id": *addresses[i].AllocationId, + "networkInterfaceID": networkInterfaceID, + }).Debug("assigning elastic IP to the instance") + err = a.tryAssignAddress(ctx, &addresses[i], networkInterfaceID, instanceID) + if err != nil { + a.logger.WithError(err).Warn("failed to assign elastic IP address") + a.logger.Debug("retrying with another address") + } else { + a.logger.WithFields(logrus.Fields{ + "instance": instanceID, + "address": *addresses[i].PublicIp, + "allocation_id": *addresses[i].AllocationId, + }).Info("elastic IP assigned to the instance") + break // break if address assigned successfully + } } - - // log available addresses IPs - ips := make([]string, 0, len(addresses)) - for _, address := range addresses { - ips = append(ips, *address.PublicIp) + if err != nil { + return errors.Wrap(err, "failed to assign elastic IP address") } - a.logger.WithField("addresses", ips).Debugf("found %d available addresses", len(addresses)) + return nil +} - // get EC2 instance - instance, err := a.instanceGetter.Get(ctx, instanceID, a.region) +func (a *awsAssigner) tryAssignAddress(ctx context.Context, address *types.Address, networkInterfaceID, instanceID string) error { + // force check if address is already assigned (reduce the chance of assigning the same address by multiple kubeip instances) + addressAssigned, err := a.forceCheckAddressAssigned(ctx, *address.AllocationId) if err != nil { - return errors.Wrapf(err, "failed to get instance %s", instanceID) + return errors.Wrapf(err, "failed to check if address %s is assigned", *address.PublicIp) + } + if addressAssigned { + return errors.Errorf("address %s is already assigned", *address.PublicIp) } + if err = a.eipAssigner.Assign(ctx, networkInterfaceID, *address.AllocationId); err != nil { + return errors.Wrapf(err, "failed to assign elastic IP %s to the instance %s", *address.PublicIp, instanceID) + } + return nil +} + +func (a *awsAssigner) getNetworkInterfaceID(instance *types.Instance) (string, error) { // get network interface ID if instance.NetworkInterfaces == nil || len(instance.NetworkInterfaces) == 0 { - return errors.Errorf("no network interfaces found for instance %s", instanceID) + return "", errors.Errorf("no network interfaces found for instance %s", *instance.InstanceId) } // get primary network interface ID with public IP address (DeviceIndex == 0) networkInterfaceID := "" @@ -224,72 +247,73 @@ func (a *awsAssigner) Assign(ctx context.Context, instanceID, _ string, filter [ } } if networkInterfaceID == "" { - return errors.Errorf("no network interfaces with public IP address found for instance %s", instanceID) + return "", errors.Errorf("no network interfaces with public IP address found for instance %s", *instance.InstanceId) } + return networkInterfaceID, nil +} - // sort addresses by orderBy field - sortAddressesByField(addresses, orderBy) - - // try to assign all available addresses until one succeeds - // due to concurrency, it is possible that another kubeip instance will assign the same address - for i := range addresses { - // force check if address is already assigned (reduce the chance of assigning the same address by multiple kubeip instances) - var addressAssigned bool - addressAssigned, err = a.forceCheckAddressAssigned(ctx, *addresses[i].AllocationId) - if err != nil { - a.logger.WithError(err).Errorf("failed to check if address %s is assigned", *addresses[i].PublicIp) - a.logger.Debug("trying next address") - continue - } - if addressAssigned { - a.logger.WithField("address", addresses[i].PublicIp).Debug("address is already assigned") - a.logger.Debug("trying next address") - continue - } - a.logger.WithFields(logrus.Fields{ - "instance": instanceID, - "address": *addresses[i].PublicIp, - "allocation_id": *addresses[i].AllocationId, - "networkInterfaceID": networkInterfaceID, - }).Debug("assigning elastic IP to the instance") - if err = a.eipAssigner.Assign(ctx, networkInterfaceID, *addresses[i].AllocationId); err != nil { - a.logger.WithFields(logrus.Fields{ - "instance": instanceID, - "address": *addresses[i].PublicIp, - "allocation_id": *addresses[i].AllocationId, - "networkInterfaceID": networkInterfaceID, - }).Debug("failed to assign elastic IP to the instance") - a.logger.Debug("trying next address") - continue - } - a.logger.WithFields(logrus.Fields{ - "instance": instanceID, - "address": *addresses[i].PublicIp, - "allocation_id": *addresses[i].AllocationId, - }).Info("elastic IP assigned to the instance") - break - } +func (a *awsAssigner) checkElasticIPAssigned(ctx context.Context, instanceID string) error { + filters := make(map[string][]string) + filters["instance-id"] = []string{instanceID} + addresses, err := a.eipLister.List(ctx, filters, true) if err != nil { - return errors.Wrap(err, "failed to assign elastic IP address") + return errors.Wrapf(err, "failed to list elastic IPs attached to instance %s", instanceID) + } + if len(addresses) > 0 { + return ErrStaticIPAlreadyAssigned } return nil } -func (a *awsAssigner) Unassign(ctx context.Context, instanceID, _ string) error { +func (a *awsAssigner) getAssignedElasticIP(ctx context.Context, instanceID string) (*types.Address, error) { // get elastic IP attached to the instance filters := make(map[string][]string) filters["instance-id"] = []string{instanceID} addresses, err := a.eipLister.List(ctx, filters, true) if err != nil { - return errors.Wrapf(err, "failed to list elastic IPs attached to instance %s", instanceID) + return nil, errors.Wrapf(err, "failed to list elastic IPs attached to instance %s", instanceID) + } + if len(addresses) == 0 { + return nil, ErrNoStaticIPAssigned + } + return &addresses[0], nil +} + +func (a *awsAssigner) getAvailableElasticIPs(ctx context.Context, filter []string, orderBy string) ([]types.Address, error) { + filters := make(map[string][]string) + for _, f := range filter { + name, values, err := parseShorthandFilter(f) + if err != nil { + return nil, errors.Wrapf(err, "failed to parse filter %s", f) + } + filters[name] = values + } + addresses, err := a.eipLister.List(ctx, filters, false) + if err != nil { + return nil, errors.Wrap(err, "failed to list available elastic IPs") } if len(addresses) == 0 { - a.logger.Infof("no elastic IP attached to instance %s", instanceID) - return nil + return nil, errors.Errorf("no available elastic IPs") + } + // sort addresses by orderBy field + sortAddressesByField(addresses, orderBy) + // log available addresses IPs + ips := make([]string, 0, len(addresses)) + for _, address := range addresses { + ips = append(ips, *address.PublicIp) } + a.logger.WithField("addresses", ips).Debugf("Found %d available addresses", len(addresses)) + return addresses, nil +} + +func (a *awsAssigner) Unassign(ctx context.Context, instanceID, _ string) error { + // get elastic IP attached to the instance + address, err := a.getAssignedElasticIP(ctx, instanceID) + if err != nil { + return errors.Wrapf(err, "check if elastic IP is assigned to instance %s", instanceID) + } // unassign elastic IP from the instance - address := addresses[0] if err = a.eipAssigner.Unassign(ctx, *address.AssociationId); err != nil { return errors.Wrap(err, "failed to unassign elastic IP") } diff --git a/internal/address/aws_test.go b/internal/address/aws_test.go index 9aab050..ba2ecb4 100644 --- a/internal/address/aws_test.go +++ b/internal/address/aws_test.go @@ -9,6 +9,7 @@ import ( "github.com/aws/aws-sdk-go-v2/service/ec2/types" "github.com/doitintl/kubeip/internal/cloud" mocks "github.com/doitintl/kubeip/mocks/cloud" + "github.com/pkg/errors" "github.com/sirupsen/logrus" ) @@ -571,6 +572,7 @@ func Test_awsAssigner_Assign(t *testing.T) { ctx: context.Background(), instanceID: "i-0abcd1234efgh5678", }, + wantErr: true, }, } for _, tt := range tests { @@ -588,3 +590,466 @@ func Test_awsAssigner_Assign(t *testing.T) { }) } } + +func Test_parseShorthandFilter(t *testing.T) { + type args struct { + filter string + } + tests := []struct { + name string + args args + want string + want1 []string + wantErr bool + }{ + { + name: "parse shorthand filter", + args: args{ + filter: "Name=tag:env,Values=val1,val2,val3", + }, + want: "tag:env", + want1: []string{"val1", "val2", "val3"}, + }, + { + name: "missing values", + args: args{ + filter: "Name=tag:env", + }, + wantErr: true, + }, + { + name: "bad filter", + args: args{ + filter: "Name=tag:env,Values=val1,val2,val3;Kind=tag", + }, + wantErr: true, + }, + { + name: "No name", + args: args{ + filter: "Kind:tag:env,Values=val1,val2,val3", + }, + wantErr: true, + }, + { + name: "No values", + args: args{ + filter: "Name=tag:env,Kind=val1,val2,val3", + }, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, got1, err := parseShorthandFilter(tt.args.filter) + if (err != nil) != tt.wantErr { + t.Errorf("parseShorthandFilter() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("parseShorthandFilter() got = %v, want %v", got, tt.want) + } + if !reflect.DeepEqual(got1, tt.want1) { + t.Errorf("parseShorthandFilter() got1 = %v, want %v", got1, tt.want1) + } + }) + } +} + +func Test_awsAssigner_tryAssignAddress(t *testing.T) { + type args struct { + address *types.Address + networkInterfaceID string + instanceID string + } + type fields struct { + region string + eipListerFn func(t *testing.T, args *args) cloud.EipLister + eipAssignerFn func(t *testing.T, args *args) cloud.EipAssigner + } + tests := []struct { + name string + fields fields + args args + wantErr bool + }{ + { + name: "assign EIP to instance", + args: args{ + address: &types.Address{ + AllocationId: aws.String("eipalloc-0abcd1234efgh5678"), + PublicIp: aws.String("100.0.0.1"), + }, + networkInterfaceID: "eni-0abcd1234efgh5678", + instanceID: "i-0abcd1234efgh5678", + }, + fields: fields{ + region: "us-east-1", + eipListerFn: func(t *testing.T, args *args) cloud.EipLister { + mock := mocks.NewEipLister(t) + mock.EXPECT().List(context.TODO(), map[string][]string{ + "allocation-id": {*args.address.AllocationId}, + }, true).Return([]types.Address{*args.address}, nil).Once() + return mock + }, + eipAssignerFn: func(t *testing.T, args *args) cloud.EipAssigner { + mock := mocks.NewEipAssigner(t) + mock.EXPECT().Assign(context.TODO(), args.networkInterfaceID, *args.address.AllocationId).Return(nil) + return mock + }, + }, + }, + { + name: "EIP already assigned to instance", + args: args{ + address: &types.Address{ + AllocationId: aws.String("eipalloc-0abcd1234efgh5678"), + PublicIp: aws.String("100.0.0.1"), + AssociationId: aws.String("eipassoc-0abcd1234efgh5678"), + }, + networkInterfaceID: "eni-0abcd1234efgh5678", + instanceID: "i-0abcd1234efgh5678", + }, + fields: fields{ + region: "us-east-1", + eipListerFn: func(t *testing.T, args *args) cloud.EipLister { + mock := mocks.NewEipLister(t) + mock.EXPECT().List(context.TODO(), map[string][]string{ + "allocation-id": {*args.address.AllocationId}, + }, true).Return([]types.Address{*args.address}, nil).Once() + return mock + }, + eipAssignerFn: func(t *testing.T, args *args) cloud.EipAssigner { + return nil + }, + }, + wantErr: true, + }, + { + name: "EIP list error", + args: args{ + address: &types.Address{ + AllocationId: aws.String("eipalloc-0abcd1234efgh5678"), + PublicIp: aws.String("100.0.0.1"), + }, + networkInterfaceID: "eni-0abcd1234efgh5678", + instanceID: "i-0abcd1234efgh5678", + }, + fields: fields{ + region: "us-east-1", + eipListerFn: func(t *testing.T, args *args) cloud.EipLister { + mock := mocks.NewEipLister(t) + mock.EXPECT().List(context.TODO(), map[string][]string{ + "allocation-id": {*args.address.AllocationId}, + }, true).Return(nil, errors.New("test-error")).Once() + return mock + }, + eipAssignerFn: func(t *testing.T, args *args) cloud.EipAssigner { + return nil + }, + }, + wantErr: true, + }, + { + name: "EIP empty list", + args: args{ + address: &types.Address{ + AllocationId: aws.String("eipalloc-0abcd1234efgh5678"), + PublicIp: aws.String("100.0.0.1"), + }, + networkInterfaceID: "eni-0abcd1234efgh5678", + instanceID: "i-0abcd1234efgh5678", + }, + fields: fields{ + region: "us-east-1", + eipListerFn: func(t *testing.T, args *args) cloud.EipLister { + mock := mocks.NewEipLister(t) + mock.EXPECT().List(context.TODO(), map[string][]string{ + "allocation-id": {*args.address.AllocationId}, + }, true).Return([]types.Address{}, nil).Once() + return mock + }, + eipAssignerFn: func(t *testing.T, args *args) cloud.EipAssigner { + mock := mocks.NewEipAssigner(t) + mock.EXPECT().Assign(context.TODO(), args.networkInterfaceID, *args.address.AllocationId).Return(nil) + return mock + }, + }, + }, + { + name: "EIP assign error", + args: args{ + address: &types.Address{ + AllocationId: aws.String("eipalloc-0abcd1234efgh5678"), + PublicIp: aws.String("100.0.0.1"), + }, + networkInterfaceID: "eni-0abcd1234efgh5678", + instanceID: "i-0abcd1234efgh5678", + }, + fields: fields{ + region: "us-east-1", + eipListerFn: func(t *testing.T, args *args) cloud.EipLister { + mock := mocks.NewEipLister(t) + mock.EXPECT().List(context.TODO(), map[string][]string{ + "allocation-id": {*args.address.AllocationId}, + }, true).Return([]types.Address{*args.address}, nil).Once() + return mock + }, + eipAssignerFn: func(t *testing.T, args *args) cloud.EipAssigner { + mock := mocks.NewEipAssigner(t) + mock.EXPECT().Assign(context.TODO(), args.networkInterfaceID, *args.address.AllocationId).Return(errors.New("test-error")) + return mock + }, + }, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + a := &awsAssigner{ + region: tt.fields.region, + eipLister: tt.fields.eipListerFn(t, &tt.args), + eipAssigner: tt.fields.eipAssignerFn(t, &tt.args), + } + if err := a.tryAssignAddress(context.TODO(), tt.args.address, tt.args.networkInterfaceID, tt.args.instanceID); (err != nil) != tt.wantErr { + t.Errorf("tryAssignAddress() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} + +func Test_awsAssigner_getNetworkInterfaceID(t *testing.T) { + type args struct { + instance *types.Instance + } + tests := []struct { + name string + args args + want string + wantErr bool + }{ + { + name: "get network interface ID", + args: args{ + instance: &types.Instance{ + NetworkInterfaces: []types.InstanceNetworkInterface{ + { + Attachment: &types.InstanceNetworkInterfaceAttachment{ + DeviceIndex: aws.Int32(0), + }, + Association: &types.InstanceNetworkInterfaceAssociation{ + PublicIp: aws.String("100.0.0.1"), + }, + NetworkInterfaceId: aws.String("eni-0abcd1234efgh5678"), + }, + }, + }, + }, + want: "eni-0abcd1234efgh5678", + }, + { + name: "no network interface ID", + args: args{ + instance: &types.Instance{ + InstanceId: aws.String("i-0abcd1234efgh5678"), + NetworkInterfaces: []types.InstanceNetworkInterface{}, + }, + }, + wantErr: true, + }, + { + name: "no public IP", + args: args{ + instance: &types.Instance{ + InstanceId: aws.String("i-0abcd1234efgh5678"), + NetworkInterfaces: []types.InstanceNetworkInterface{ + { + Attachment: &types.InstanceNetworkInterfaceAttachment{ + DeviceIndex: aws.Int32(0), + }, + Association: &types.InstanceNetworkInterfaceAssociation{ + PublicIp: nil, + }, + }, + }, + }, + }, + wantErr: true, + }, + { + name: "multiple network interfaces", + args: args{ + instance: &types.Instance{ + NetworkInterfaces: []types.InstanceNetworkInterface{ + { + Attachment: &types.InstanceNetworkInterfaceAttachment{ + DeviceIndex: aws.Int32(1), + }, + Association: &types.InstanceNetworkInterfaceAssociation{ + PublicIp: aws.String("100.0.0.1"), + }, + NetworkInterfaceId: aws.String("eni-0abcd1234efgh5678"), + }, + { + Attachment: &types.InstanceNetworkInterfaceAttachment{ + DeviceIndex: aws.Int32(0), + }, + Association: &types.InstanceNetworkInterfaceAssociation{ + PublicIp: aws.String("100.0.0.2"), + }, + NetworkInterfaceId: aws.String("eni-0abcd1234efgh5679"), + }, + }, + }, + }, + want: "eni-0abcd1234efgh5679", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + a := &awsAssigner{} + got, err := a.getNetworkInterfaceID(tt.args.instance) + if (err != nil) != tt.wantErr { + t.Errorf("getNetworkInterfaceID() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("getNetworkInterfaceID() got = %v, want %v", got, tt.want) + } + }) + } +} + +func Test_awsAssigner_getAssignedElasticIP(t *testing.T) { + type args struct { + instanceID string + } + type fields struct { + region string + eipListerFn func(t *testing.T, args *args) cloud.EipLister + } + tests := []struct { + name string + fields fields + args args + want *types.Address + wantErr bool + }{ + { + name: "get assigned EIP", + args: args{ + instanceID: "i-0abcd1234efgh5678", + }, + fields: fields{ + region: "us-east-1", + eipListerFn: func(t *testing.T, args *args) cloud.EipLister { + mock := mocks.NewEipLister(t) + mock.EXPECT().List(context.TODO(), map[string][]string{ + "instance-id": {args.instanceID}, + }, true).Return([]types.Address{ + { + AllocationId: aws.String("eipalloc-0abcd1234efgh5678"), + PublicIp: aws.String("100.0.0.1"), + }, + }, nil).Once() + return mock + }, + }, + want: &types.Address{ + AllocationId: aws.String("eipalloc-0abcd1234efgh5678"), + PublicIp: aws.String("100.0.0.1"), + }, + }, + { + name: "no assigned EIP", + args: args{ + instanceID: "i-0abcd1234efgh5678", + }, + fields: fields{ + region: "us-east-1", + eipListerFn: func(t *testing.T, args *args) cloud.EipLister { + mock := mocks.NewEipLister(t) + mock.EXPECT().List(context.TODO(), map[string][]string{ + "instance-id": {args.instanceID}, + }, true).Return([]types.Address{}, nil).Once() + return mock + }, + }, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + a := &awsAssigner{ + region: tt.fields.region, + eipLister: tt.fields.eipListerFn(t, &tt.args), + } + got, err := a.getAssignedElasticIP(context.TODO(), tt.args.instanceID) + if (err != nil) != tt.wantErr { + t.Errorf("getAssignedElasticIP() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("getAssignedElasticIP() got = %v, want %v", got, tt.want) + } + }) + } +} + +func Test_awsAssigner_Unassign(t *testing.T) { + type args struct { + instanceID string + } + type fields struct { + region string + eipListerFn func(t *testing.T, args *args) cloud.EipLister + eipAssignerFn func(t *testing.T, args *args) cloud.EipAssigner + } + tests := []struct { + name string + fields fields + args args + wantErr bool + }{ + { + name: "unassign EIP from instance", + args: args{ + instanceID: "i-0abcd1234efgh5678", + }, + fields: fields{ + region: "us-east-1", + eipListerFn: func(t *testing.T, args *args) cloud.EipLister { + mock := mocks.NewEipLister(t) + mock.EXPECT().List(context.TODO(), map[string][]string{ + "instance-id": {args.instanceID}, + }, true).Return([]types.Address{ + { + AllocationId: aws.String("eipalloc-0abcd1234efgh5678"), + AssociationId: aws.String("eipassoc-0abcd1234efgh5678"), + PublicIp: aws.String("100.0.0.1"), + }, + }, nil).Once() + return mock + }, + eipAssignerFn: func(t *testing.T, args *args) cloud.EipAssigner { + mock := mocks.NewEipAssigner(t) + mock.EXPECT().Unassign(context.TODO(), "eipassoc-0abcd1234efgh5678").Return(nil) + return mock + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + a := &awsAssigner{ + region: tt.fields.region, + logger: logrus.NewEntry(logrus.New()), + eipLister: tt.fields.eipListerFn(t, &tt.args), + eipAssigner: tt.fields.eipAssignerFn(t, &tt.args), + } + if err := a.Unassign(context.TODO(), tt.args.instanceID, ""); (err != nil) != tt.wantErr { + t.Errorf("Unassign() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} diff --git a/internal/address/gcp.go b/internal/address/gcp.go index f824eb0..a463e2a 100644 --- a/internal/address/gcp.go +++ b/internal/address/gcp.go @@ -212,20 +212,9 @@ func (a *gcpAssigner) CheckAddressAssigned(region, addressName string) (bool, er func (a *gcpAssigner) Assign(ctx context.Context, instanceID, zone string, filter []string, orderBy string) error { // check if instance already has a public static IP address assigned - instance, err := a.instanceGetter.Get(a.project, zone, instanceID) + instance, err := a.checkStaticIPAssigned(zone, instanceID) if err != nil { - return errors.Wrapf(err, "failed to get instance %s", instanceID) - } - assigned, err := a.listAddresses(nil, "", inUseStatus) - if err != nil { - return errors.Wrap(err, "failed to list assigned addresses") - } - // create a map of users for quick lookup - users := a.createUserMap(assigned) - // check if the instance's self link is in the list of users - if _, ok := users[instance.SelfLink]; ok { - a.logger.WithField("instance", instanceID).Infof("instance already has a public IP address assigned") - return nil + return errors.Wrapf(err, "check if static public IP is already assigned to instance %s", instanceID) } // get available reserved public IP addresses @@ -270,6 +259,24 @@ func (a *gcpAssigner) Assign(ctx context.Context, instanceID, zone string, filte return nil } +func (a *gcpAssigner) checkStaticIPAssigned(zone, instanceID string) (*compute.Instance, error) { + instance, err := a.instanceGetter.Get(a.project, zone, instanceID) + if err != nil { + return nil, errors.Wrapf(err, "failed to get instance %s", instanceID) + } + assigned, err := a.listAddresses(nil, "", inUseStatus) + if err != nil { + return nil, errors.Wrap(err, "failed to list assigned addresses") + } + // create a map of users for quick lookup + users := a.createUserMap(assigned) + // check if the instance's self link is in the list of users + if _, ok := users[instance.SelfLink]; ok { + return nil, ErrStaticIPAlreadyAssigned + } + return instance, nil +} + func (a *gcpAssigner) listAddresses(filter []string, orderBy, status string) ([]*compute.Address, error) { call := a.lister.List(a.project, a.region) // Initialize filters with known filters @@ -319,6 +326,9 @@ func (a *gcpAssigner) Unassign(ctx context.Context, instanceID, zone string) err if err != nil { return errors.Wrap(err, "failed to list assigned addresses") } + if len(assigned) == 0 { + return ErrNoStaticIPAssigned + } // create a map of users for quick lookup users := a.createUserMap(assigned)