diff --git a/CHANGELOG.md b/CHANGELOG.md index 76131e359..8e1aa8265 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Loadbalancer for control-plane recovery - K8s conformance mode - Local cluster creation based on QEMU +- GCP instances groups are now spread across multiple zones for high availability per default. ### Changed diff --git a/internal/cloud/gcp/api.go b/internal/cloud/gcp/api.go index 6e2262165..d833bf486 100644 --- a/internal/cloud/gcp/api.go +++ b/internal/cloud/gcp/api.go @@ -17,7 +17,7 @@ import ( type instanceAPI interface { Get(ctx context.Context, req *computepb.GetInstanceRequest, opts ...gax.CallOption) (*computepb.Instance, error) - List(ctx context.Context, req *computepb.ListInstancesRequest, opts ...gax.CallOption) InstanceIterator + AggregatedList(ctx context.Context, req *computepb.AggregatedListInstancesRequest, opts ...gax.CallOption) InstancesScopedListPairIterator SetMetadata(ctx context.Context, req *computepb.SetMetadataInstanceRequest, opts ...gax.CallOption) (*compute.Operation, error) Close() error } @@ -44,8 +44,8 @@ type Operation interface { Proto() *computepb.Operation } -type InstanceIterator interface { - Next() (*computepb.Instance, error) +type InstancesScopedListPairIterator interface { + Next() (compute.InstancesScopedListPair, error) } type SubnetworkIterator interface { diff --git a/internal/cloud/gcp/ccm.go b/internal/cloud/gcp/ccm.go index b3d43d9b6..99efbd238 100644 --- a/internal/cloud/gcp/ccm.go +++ b/internal/cloud/gcp/ccm.go @@ -74,11 +74,13 @@ func (c *CloudControllerManager) ExtraArgs() []string { func (c *CloudControllerManager) ConfigMaps() (kubernetes.ConfigMaps, error) { // GCP CCM expects cloud config to contain the GCP project-id and other configuration. // reference: https://github.com/kubernetes/cloud-provider-gcp/blob/master/cluster/gce/gci/configure-helper.sh#L791-L892 + // reference: https://github.com/kubernetes/cloud-provider-gcp/blob/d2b22ad6f19d3b11e9c6e2f308dd69a2bab6d660/providers/gce/gce.go#L183-L216 var config strings.Builder config.WriteString("[global]\n") config.WriteString(fmt.Sprintf("project-id = %s\n", c.projectID)) config.WriteString("use-metadata-server = true\n") config.WriteString(fmt.Sprintf("node-tags = constellation-%s\n", c.uid)) + config.WriteString("regional = true\n") return kubernetes.ConfigMaps{ &k8s.ConfigMap{ diff --git a/internal/cloud/gcp/ccm_test.go b/internal/cloud/gcp/ccm_test.go index 7fbb1a1a5..0ed811226 100644 --- a/internal/cloud/gcp/ccm_test.go +++ b/internal/cloud/gcp/ccm_test.go @@ -43,6 +43,7 @@ func TestConfigMaps(t *testing.T) { project-id = project-id use-metadata-server = true node-tags = constellation-UID +regional = true `, }, }, diff --git a/internal/cloud/gcp/client.go b/internal/cloud/gcp/client.go index 7a0565f91..22846089a 100644 --- a/internal/cloud/gcp/client.go +++ b/internal/cloud/gcp/client.go @@ -60,39 +60,39 @@ func NewClient(ctx context.Context) (*Client, error) { } // RetrieveInstances returns list of instances including their ips and metadata. -func (c *Client) RetrieveInstances(ctx context.Context, project, zone string) ([]metadata.InstanceMetadata, error) { +func (c *Client) RetrieveInstances(ctx context.Context, project string) ([]metadata.InstanceMetadata, error) { uid, err := c.UID() if err != nil { return nil, err } - req := &computepb.ListInstancesRequest{ + req := &computepb.AggregatedListInstancesRequest{ Project: project, - Zone: zone, + Filter: proto.String(fmt.Sprintf("name eq \".+-%s-.+\"", uid)), // filter by constellation UID } - instanceIterator := c.instanceAPI.List(ctx, req) + listPairIterator := c.instanceAPI.AggregatedList(ctx, req) - instances := []metadata.InstanceMetadata{} + instancesMetadataList := []metadata.InstanceMetadata{} for { - resp, err := instanceIterator.Next() + resp, err := listPairIterator.Next() if err == iterator.Done { break } if err != nil { - return nil, fmt.Errorf("retrieving instance list from compute API client: %w", err) + return nil, fmt.Errorf("retrieving instances scoped list pair from compute API client: %w", err) } - metadata := extractInstanceMetadata(resp.Metadata, "", false) - // skip instances not belonging to the current constellation - if instanceUID, ok := metadata[constellationUIDMetadataKey]; !ok || instanceUID != uid { - continue - } - instance, err := convertToCoreInstance(resp, project, zone) - if err != nil { - return nil, err + if resp.Value == nil { + return nil, fmt.Errorf("retrieving instances scoped list pair from compute API client returned invalid results") } - instances = append(instances, instance) + for _, instance := range resp.Value.Instances { + instanceMetadata, err := convertToMetadataInstance(instance, project) + if err != nil { + return nil, fmt.Errorf("extracting metadata of instance: %w", err) + } + instancesMetadataList = append(instancesMetadataList, instanceMetadata) + } } - return instances, nil + return instancesMetadataList, nil } // RetrieveInstance returns a an instance including ips and metadata. @@ -102,7 +102,7 @@ func (c *Client) RetrieveInstance(ctx context.Context, project, zone, instanceNa return metadata.InstanceMetadata{}, err } - return convertToCoreInstance(instance, project, zone) + return convertToMetadataInstance(instance, project) } // RetrieveProjectID retrieves the GCP projectID containing the current instance. @@ -376,10 +376,29 @@ func extractSSHKeys(metadata map[string]string) map[string][]string { return keys } -// convertToCoreInstance converts a *computepb.Instance to a core.Instance. -func convertToCoreInstance(in *computepb.Instance, project string, zone string) (metadata.InstanceMetadata, error) { +func extractZone(in *computepb.Instance) (string, error) { + if in == nil { + return "", errors.New("instance is nil") + } + if in.Zone == nil { + return "", errors.New("instance zone is nil") + } + zoneURL := *in.Zone + zoneParts := strings.Split(zoneURL, "/") + if zoneParts[len(zoneParts)-2] != "zones" { + return "", fmt.Errorf("invalid zone URL: %s", zoneURL) + } + return zoneParts[len(zoneParts)-1], nil +} + +// convertToMetadataInstance converts a *computepb.Instance to a core.Instance. +func convertToMetadataInstance(in *computepb.Instance, project string) (metadata.InstanceMetadata, error) { if in.Name == nil { - return metadata.InstanceMetadata{}, fmt.Errorf("retrieving instance from compute API client returned invalid instance Name: %v", in.Name) + return metadata.InstanceMetadata{}, fmt.Errorf("retrieving instance from compute API client returned invalid instance name: %v", in.Name) + } + zone, err := extractZone(in) + if err != nil { + return metadata.InstanceMetadata{}, fmt.Errorf("extracting zone from instance: %w", err) } mdata := extractInstanceMetadata(in.Metadata, "", false) return metadata.InstanceMetadata{ diff --git a/internal/cloud/gcp/client_test.go b/internal/cloud/gcp/client_test.go index b2f594ac0..6e99f7cd1 100644 --- a/internal/cloud/gcp/client_test.go +++ b/internal/cloud/gcp/client_test.go @@ -33,56 +33,54 @@ func TestMain(m *testing.M) { func TestRetrieveInstances(t *testing.T) { uid := "1234" someErr := errors.New("failed") - newTestIter := func() *stubInstanceIterator { - return &stubInstanceIterator{ - instances: []*computepb.Instance{ + newTestInstance := func() *computepb.Instance { + return &computepb.Instance{ + Name: proto.String("someInstance"), + Zone: proto.String("/project/someProject/zones/someZone"), + Metadata: &computepb.Metadata{ + Items: []*computepb.Items{ + { + Key: proto.String("ssh-keys"), + Value: proto.String("bob:ssh-rsa bobskey"), + }, + { + Key: proto.String("key-2"), + Value: proto.String("value-2"), + }, + { + Key: proto.String(constellationUIDMetadataKey), + Value: proto.String(uid), + }, + { + Key: proto.String(roleMetadataKey), + Value: proto.String(role.ControlPlane.String()), + }, + }, + }, + NetworkInterfaces: []*computepb.NetworkInterface{ { - Name: proto.String("someInstance"), - Metadata: &computepb.Metadata{ - Items: []*computepb.Items{ - { - Key: proto.String("ssh-keys"), - Value: proto.String("bob:ssh-rsa bobskey"), - }, - { - Key: proto.String("key-2"), - Value: proto.String("value-2"), - }, - { - Key: proto.String(constellationUIDMetadataKey), - Value: proto.String(uid), - }, - { - Key: proto.String(roleMetadataKey), - Value: proto.String(role.ControlPlane.String()), - }, - }, - }, - NetworkInterfaces: []*computepb.NetworkInterface{ - { - Name: proto.String("nic0"), - NetworkIP: proto.String("192.0.2.0"), - AliasIpRanges: []*computepb.AliasIpRange{{IpCidrRange: proto.String("192.0.2.0/16")}}, - AccessConfigs: []*computepb.AccessConfig{{NatIP: proto.String("192.0.2.1")}}, - }, - }, + Name: proto.String("nic0"), + NetworkIP: proto.String("192.0.2.0"), + AliasIpRanges: []*computepb.AliasIpRange{{IpCidrRange: proto.String("192.0.2.0/16")}}, + AccessConfigs: []*computepb.AccessConfig{{NatIP: proto.String("192.0.2.1")}}, }, }, } } testCases := map[string]struct { - client stubInstancesClient - metadata stubMetadataClient - instanceIter *stubInstanceIterator - instanceIterMutator func(*stubInstanceIterator) - wantInstances []metadata.InstanceMetadata - wantErr bool + client stubInstancesClient + metadata stubMetadataClient + instanceIterInstance *computepb.Instance + iterInstanceMutator func(*computepb.Instance) + instanceIterErr error + wantInstances []metadata.InstanceMetadata + wantErr bool }{ "retrieve works": { - client: stubInstancesClient{}, - metadata: stubMetadataClient{InstanceValue: uid}, - instanceIter: newTestIter(), + client: stubInstancesClient{}, + metadata: stubMetadataClient{InstanceValue: uid}, + instanceIterInstance: newTestInstance(), wantInstances: []metadata.InstanceMetadata{ { Name: "someInstance", @@ -96,17 +94,16 @@ func TestRetrieveInstances(t *testing.T) { }, }, "instance name is null": { - client: stubInstancesClient{}, - metadata: stubMetadataClient{InstanceValue: uid}, - instanceIter: newTestIter(), - instanceIterMutator: func(sii *stubInstanceIterator) { sii.instances[0].Name = nil }, - wantErr: true, + client: stubInstancesClient{}, + metadata: stubMetadataClient{InstanceValue: uid}, + instanceIterInstance: &computepb.Instance{Name: nil}, + wantErr: true, }, "no instance with network ip": { - client: stubInstancesClient{}, - metadata: stubMetadataClient{InstanceValue: uid}, - instanceIter: newTestIter(), - instanceIterMutator: func(sii *stubInstanceIterator) { sii.instances[0].NetworkInterfaces = nil }, + client: stubInstancesClient{}, + metadata: stubMetadataClient{InstanceValue: uid}, + instanceIterInstance: newTestInstance(), + iterInstanceMutator: func(i *computepb.Instance) { i.NetworkInterfaces = nil }, wantInstances: []metadata.InstanceMetadata{ { Name: "someInstance", @@ -120,10 +117,10 @@ func TestRetrieveInstances(t *testing.T) { }, }, "network ip is nil": { - client: stubInstancesClient{}, - metadata: stubMetadataClient{InstanceValue: uid}, - instanceIter: newTestIter(), - instanceIterMutator: func(sii *stubInstanceIterator) { sii.instances[0].NetworkInterfaces[0].NetworkIP = nil }, + client: stubInstancesClient{}, + metadata: stubMetadataClient{InstanceValue: uid}, + instanceIterInstance: newTestInstance(), + iterInstanceMutator: func(i *computepb.Instance) { i.NetworkInterfaces[0].NetworkIP = nil }, wantInstances: []metadata.InstanceMetadata{ { Name: "someInstance", @@ -136,24 +133,17 @@ func TestRetrieveInstances(t *testing.T) { }, }, }, - "constellation id is not set": { - client: stubInstancesClient{}, - metadata: stubMetadataClient{InstanceValue: uid}, - instanceIter: newTestIter(), - instanceIterMutator: func(sii *stubInstanceIterator) { sii.instances[0].Metadata.Items[2].Key = proto.String("") }, - wantInstances: []metadata.InstanceMetadata{}, - }, "constellation retrieval fails": { - client: stubInstancesClient{}, - metadata: stubMetadataClient{InstanceErr: someErr}, - instanceIter: newTestIter(), - wantErr: true, + client: stubInstancesClient{}, + metadata: stubMetadataClient{InstanceErr: someErr}, + instanceIterInstance: newTestInstance(), + wantErr: true, }, "role is not set": { - client: stubInstancesClient{}, - metadata: stubMetadataClient{InstanceValue: uid}, - instanceIter: newTestIter(), - instanceIterMutator: func(sii *stubInstanceIterator) { sii.instances[0].Metadata.Items[3].Key = proto.String("") }, + client: stubInstancesClient{}, + metadata: stubMetadataClient{InstanceValue: uid}, + instanceIterInstance: newTestInstance(), + iterInstanceMutator: func(i *computepb.Instance) { i.Metadata.Items[3].Key = proto.String("") }, wantInstances: []metadata.InstanceMetadata{ { Name: "someInstance", @@ -167,10 +157,10 @@ func TestRetrieveInstances(t *testing.T) { }, }, "instance iterator Next() errors": { - client: stubInstancesClient{}, - metadata: stubMetadataClient{InstanceValue: uid}, - instanceIter: &stubInstanceIterator{nextErr: someErr}, - wantErr: true, + client: stubInstancesClient{}, + metadata: stubMetadataClient{InstanceValue: uid}, + instanceIterErr: someErr, + wantErr: true, }, } @@ -179,16 +169,18 @@ func TestRetrieveInstances(t *testing.T) { assert := assert.New(t) require := require.New(t) - if tc.instanceIterMutator != nil { - tc.instanceIterMutator(tc.instanceIter) + if tc.iterInstanceMutator != nil { + tc.iterInstanceMutator(tc.instanceIterInstance) } - tc.client.ListInstanceIterator = tc.instanceIter + tc.client.ListInstanceIterator = newStubInstancesScopedListPairIterator( + []*computepb.Instance{tc.instanceIterInstance}, tc.instanceIterErr, + ) client := Client{ instanceAPI: tc.client, metadataAPI: tc.metadata, } - instances, err := client.RetrieveInstances(context.Background(), "someProject", "someZone") + instances, err := client.RetrieveInstances(context.Background(), "someProject") if tc.wantErr { assert.Error(err) @@ -204,6 +196,7 @@ func TestRetrieveInstance(t *testing.T) { newTestInstance := func() *computepb.Instance { return &computepb.Instance{ Name: proto.String("someInstance"), + Zone: proto.String("/project/someProject/zones/someZone"), Metadata: &computepb.Metadata{ Items: []*computepb.Items{ { @@ -948,22 +941,34 @@ func TestFetchSSHKeys(t *testing.T) { } } -type stubInstanceIterator struct { - instances []*computepb.Instance - nextErr error +type stubInstancesScopedListPairIterator struct { + pairs []compute.InstancesScopedListPair + nextErr error internalCounter int } -func (i *stubInstanceIterator) Next() (*computepb.Instance, error) { +func newStubInstancesScopedListPairIterator(instances []*computepb.Instance, nextErr error) *stubInstancesScopedListPairIterator { + return &stubInstancesScopedListPairIterator{ + pairs: []compute.InstancesScopedListPair{ + { + Key: "zone", + Value: &computepb.InstancesScopedList{Instances: instances}, + }, + }, + nextErr: nextErr, + } +} + +func (i *stubInstancesScopedListPairIterator) Next() (compute.InstancesScopedListPair, error) { if i.nextErr != nil { - return nil, i.nextErr + return compute.InstancesScopedListPair{}, i.nextErr } - if i.internalCounter >= len(i.instances) { + if i.internalCounter >= len(i.pairs) { i.internalCounter = 0 - return nil, iterator.Done + return compute.InstancesScopedListPair{}, iterator.Done } - resp := i.instances[i.internalCounter] + resp := i.pairs[i.internalCounter] i.internalCounter++ return resp, nil } @@ -971,7 +976,7 @@ func (i *stubInstanceIterator) Next() (*computepb.Instance, error) { type stubInstancesClient struct { GetInstance *computepb.Instance GetErr error - ListInstanceIterator InstanceIterator + ListInstanceIterator InstancesScopedListPairIterator SetMetadataOperation *compute.Operation SetMetadataErr error CloseErr error @@ -981,7 +986,7 @@ func (s stubInstancesClient) Get(ctx context.Context, req *computepb.GetInstance return s.GetInstance, s.GetErr } -func (s stubInstancesClient) List(ctx context.Context, req *computepb.ListInstancesRequest, opts ...gax.CallOption) InstanceIterator { +func (s stubInstancesClient) AggregatedList(ctx context.Context, req *computepb.AggregatedListInstancesRequest, opts ...gax.CallOption) InstancesScopedListPairIterator { return s.ListInstanceIterator } diff --git a/internal/cloud/gcp/metadata.go b/internal/cloud/gcp/metadata.go index 2b92ea6ae..954e4b085 100644 --- a/internal/cloud/gcp/metadata.go +++ b/internal/cloud/gcp/metadata.go @@ -19,7 +19,7 @@ type API interface { // UID retrieves the current instances uid. UID() (string, error) // RetrieveInstances retrieves a list of all accessible GCP instances with their metadata. - RetrieveInstances(ctx context.Context, project, zone string) ([]metadata.InstanceMetadata, error) + RetrieveInstances(ctx context.Context, project string) ([]metadata.InstanceMetadata, error) // RetrieveInstances retrieves a single GCP instances with its metadata. RetrieveInstance(ctx context.Context, project, zone, instanceName string) (metadata.InstanceMetadata, error) // RetrieveInstanceMetadata retrieves the GCP instance metadata of the current instance. @@ -58,11 +58,7 @@ func (m *Metadata) List(ctx context.Context) ([]metadata.InstanceMetadata, error if err != nil { return nil, err } - zone, err := m.api.RetrieveZone() - if err != nil { - return nil, err - } - instances, err := m.api.RetrieveInstances(ctx, project, zone) + instances, err := m.api.RetrieveInstances(ctx, project) if err != nil { return nil, fmt.Errorf("retrieving instances list from GCP api: %w", err) } diff --git a/internal/cloud/gcp/metadata_test.go b/internal/cloud/gcp/metadata_test.go index 14e13ad7b..aa30be166 100644 --- a/internal/cloud/gcp/metadata_test.go +++ b/internal/cloud/gcp/metadata_test.go @@ -72,13 +72,6 @@ func TestList(t *testing.T) { instancesGenerator: instancesGenerator, wantErr: true, }, - "zone retrieval error is detected": { - client: stubGCPClient{ - retrieveZoneErr: err, - }, - instancesGenerator: instancesGenerator, - wantErr: true, - }, } for name, tc := range testCases { @@ -269,7 +262,7 @@ type stubGCPClient struct { unsetMetadataKeys []string } -func (s *stubGCPClient) RetrieveInstances(ctx context.Context, project, zone string) ([]metadata.InstanceMetadata, error) { +func (s *stubGCPClient) RetrieveInstances(ctx context.Context, project string) ([]metadata.InstanceMetadata, error) { return s.retrieveInstancesValues, s.retrieveInstancesErr } diff --git a/internal/cloud/gcp/wrappers.go b/internal/cloud/gcp/wrappers.go index cc51239b4..73ad67b5d 100644 --- a/internal/cloud/gcp/wrappers.go +++ b/internal/cloud/gcp/wrappers.go @@ -23,10 +23,10 @@ func (c *instanceClient) Close() error { return c.InstancesClient.Close() } -func (c *instanceClient) List(ctx context.Context, req *computepb.ListInstancesRequest, +func (c *instanceClient) AggregatedList(ctx context.Context, req *computepb.AggregatedListInstancesRequest, opts ...gax.CallOption, -) InstanceIterator { - return c.InstancesClient.List(ctx, req) +) InstancesScopedListPairIterator { + return c.InstancesClient.AggregatedList(ctx, req) } type subnetworkClient struct { diff --git a/operators/constellation-node-operator/internal/gcp/client/api.go b/operators/constellation-node-operator/internal/gcp/client/api.go index e778dfa0b..2205fa366 100644 --- a/operators/constellation-node-operator/internal/gcp/client/api.go +++ b/operators/constellation-node-operator/internal/gcp/client/api.go @@ -38,15 +38,22 @@ type instanceTemplateAPI interface { type instanceGroupManagersAPI interface { Close() error - Get(ctx context.Context, req *computepb.GetInstanceGroupManagerRequest, - opts ...gax.CallOption) (*computepb.InstanceGroupManager, error) AggregatedList(ctx context.Context, req *computepb.AggregatedListInstanceGroupManagersRequest, opts ...gax.CallOption) InstanceGroupManagerScopedListIterator - SetInstanceTemplate(ctx context.Context, req *computepb.SetInstanceTemplateInstanceGroupManagerRequest, +} + +type regionInstanceGroupManagersAPI interface { + Close() error + Get(ctx context.Context, req *computepb.GetRegionInstanceGroupManagerRequest, + opts ...gax.CallOption) (*computepb.InstanceGroupManager, error) + ListManagedInstances(ctx context.Context, + req *computepb.ListManagedInstancesRegionInstanceGroupManagersRequest, + opts ...gax.CallOption) ManagedInstanceIterator + SetInstanceTemplate(ctx context.Context, req *computepb.SetInstanceTemplateRegionInstanceGroupManagerRequest, opts ...gax.CallOption) (Operation, error) - CreateInstances(ctx context.Context, req *computepb.CreateInstancesInstanceGroupManagerRequest, + CreateInstances(ctx context.Context, req *computepb.CreateInstancesRegionInstanceGroupManagerRequest, opts ...gax.CallOption) (Operation, error) - DeleteInstances(ctx context.Context, req *computepb.DeleteInstancesInstanceGroupManagerRequest, + DeleteInstances(ctx context.Context, req *computepb.DeleteInstancesRegionInstanceGroupManagerRequest, opts ...gax.CallOption) (Operation, error) } @@ -66,6 +73,14 @@ type InstanceGroupManagerScopedListIterator interface { Next() (compute.InstanceGroupManagersScopedListPair, error) } +type ManagedInstanceIterator interface { + Next() (*computepb.ManagedInstance, error) +} + +type InstanceGroupIterator interface { + Next() (*computepb.InstanceGroup, error) +} + type prng interface { // Intn returns, as an int, a non-negative pseudo-random number in the half-open interval [0,n). It panics if n <= 0. Intn(n int) int diff --git a/operators/constellation-node-operator/internal/gcp/client/client.go b/operators/constellation-node-operator/internal/gcp/client/client.go index f995feb14..9af88f0a9 100644 --- a/operators/constellation-node-operator/internal/gcp/client/client.go +++ b/operators/constellation-node-operator/internal/gcp/client/client.go @@ -23,6 +23,7 @@ type Client struct { instanceAPI instanceTemplateAPI instanceGroupManagersAPI + regionInstanceGroupManagersAPI diskAPI // prng is a pseudo-random number generator seeded with time. Not used for security. prng @@ -40,7 +41,6 @@ func New(ctx context.Context, configPath string) (*Client, error) { if err != nil { return nil, err } - closers = append(closers, projectAPI) insAPI, err := compute.NewInstancesRESTClient(ctx) if err != nil { @@ -60,19 +60,26 @@ func New(ctx context.Context, configPath string) (*Client, error) { return nil, err } closers = append(closers, groupAPI) + regionGroupAPI, err := compute.NewRegionInstanceGroupManagersRESTClient(ctx) + if err != nil { + _ = closeAll(closers) + return nil, err + } + closers = append(closers, regionGroupAPI) diskAPI, err := compute.NewDisksRESTClient(ctx) if err != nil { _ = closeAll(closers) return nil, err } return &Client{ - projectID: projectID, - projectAPI: projectAPI, - instanceAPI: insAPI, - instanceTemplateAPI: &instanceTemplateClient{templAPI}, - instanceGroupManagersAPI: &instanceGroupManagersClient{groupAPI}, - diskAPI: diskAPI, - prng: rand.New(rand.NewSource(int64(time.Now().Nanosecond()))), + projectID: projectID, + projectAPI: projectAPI, + instanceAPI: insAPI, + instanceTemplateAPI: &instanceTemplateClient{templAPI}, + instanceGroupManagersAPI: &instanceGroupManagersClient{groupAPI}, + regionInstanceGroupManagersAPI: ®ionInstanceGroupManagersClient{regionGroupAPI}, + diskAPI: diskAPI, + prng: rand.New(rand.NewSource(int64(time.Now().Nanosecond()))), }, nil } @@ -82,7 +89,7 @@ func (c *Client) Close() error { c.projectAPI, c.instanceAPI, c.instanceTemplateAPI, - c.instanceGroupManagersAPI, + c.regionInstanceGroupManagersAPI, c.diskAPI, } return closeAll(closers) diff --git a/operators/constellation-node-operator/internal/gcp/client/client_test.go b/operators/constellation-node-operator/internal/gcp/client/client_test.go index 4b78253a8..134e474ba 100644 --- a/operators/constellation-node-operator/internal/gcp/client/client_test.go +++ b/operators/constellation-node-operator/internal/gcp/client/client_test.go @@ -8,6 +8,7 @@ package client import ( "context" + "fmt" compute "cloud.google.com/go/compute/apiv1" "github.com/googleapis/gax-go/v2" @@ -84,24 +85,14 @@ func (a stubInstanceTemplateAPI) Insert(ctx context.Context, req *computepb.Inse } type stubInstanceGroupManagersAPI struct { - instanceGroupManager *computepb.InstanceGroupManager - getErr error - aggregatedListErr error - setInstanceTemplateErr error - createInstancesErr error - deleteInstancesErr error + instanceGroupManager *computepb.InstanceGroupManager + aggregatedListErr error } func (a stubInstanceGroupManagersAPI) Close() error { return nil } -func (a stubInstanceGroupManagersAPI) Get(ctx context.Context, req *computepb.GetInstanceGroupManagerRequest, - opts ...gax.CallOption, -) (*computepb.InstanceGroupManager, error) { - return a.instanceGroupManager, a.getErr -} - func (a stubInstanceGroupManagersAPI) AggregatedList(ctx context.Context, req *computepb.AggregatedListInstanceGroupManagersRequest, opts ...gax.CallOption, ) InstanceGroupManagerScopedListIterator { @@ -120,7 +111,36 @@ func (a stubInstanceGroupManagersAPI) AggregatedList(ctx context.Context, req *c } } -func (a stubInstanceGroupManagersAPI) SetInstanceTemplate(ctx context.Context, req *computepb.SetInstanceTemplateInstanceGroupManagerRequest, +type stubRegionInstanceGroupManagersAPI struct { + instanceGroupManager *computepb.InstanceGroupManager + managedInstance *computepb.ManagedInstance + getErr error + listManagedInstancesErr error + setInstanceTemplateErr error + createInstancesErr error + deleteInstancesErr error +} + +func (a *stubRegionInstanceGroupManagersAPI) Close() error { + return nil +} + +func (a *stubRegionInstanceGroupManagersAPI) Get(ctx context.Context, req *computepb.GetRegionInstanceGroupManagerRequest, + opts ...gax.CallOption, +) (*computepb.InstanceGroupManager, error) { + return a.instanceGroupManager, a.getErr +} + +func (a *stubRegionInstanceGroupManagersAPI) ListManagedInstances(ctx context.Context, req *computepb.ListManagedInstancesRegionInstanceGroupManagersRequest, + opts ...gax.CallOption, +) ManagedInstanceIterator { + return &stubManagedInstanceIterator{ + instances: []*computepb.ManagedInstance{a.managedInstance}, + nextErr: a.listManagedInstancesErr, + } +} + +func (a *stubRegionInstanceGroupManagersAPI) SetInstanceTemplate(ctx context.Context, req *computepb.SetInstanceTemplateRegionInstanceGroupManagerRequest, opts ...gax.CallOption, ) (Operation, error) { return &stubOperation{ @@ -130,9 +150,15 @@ func (a stubInstanceGroupManagersAPI) SetInstanceTemplate(ctx context.Context, r }, a.setInstanceTemplateErr } -func (a stubInstanceGroupManagersAPI) CreateInstances(ctx context.Context, req *computepb.CreateInstancesInstanceGroupManagerRequest, +func (a *stubRegionInstanceGroupManagersAPI) CreateInstances(ctx context.Context, req *computepb.CreateInstancesRegionInstanceGroupManagerRequest, opts ...gax.CallOption, ) (Operation, error) { + if len(req.RegionInstanceGroupManagersCreateInstancesRequestResource.Instances) != 0 { + name := *req.RegionInstanceGroupManagersCreateInstancesRequestResource.Instances[0].Name + a.managedInstance = &computepb.ManagedInstance{ + Instance: proto.String(fmt.Sprintf("https://www.googleapis.com/compute/v1/projects/project/zones/zone/instances/%s", name)), + } + } return &stubOperation{ &computepb.Operation{ Name: proto.String("name"), @@ -140,7 +166,7 @@ func (a stubInstanceGroupManagersAPI) CreateInstances(ctx context.Context, req * }, a.createInstancesErr } -func (a stubInstanceGroupManagersAPI) DeleteInstances(ctx context.Context, req *computepb.DeleteInstancesInstanceGroupManagerRequest, +func (a *stubRegionInstanceGroupManagersAPI) DeleteInstances(ctx context.Context, req *computepb.DeleteInstancesRegionInstanceGroupManagerRequest, opts ...gax.CallOption, ) (Operation, error) { if a.deleteInstancesErr != nil { @@ -202,3 +228,22 @@ func (i *stubInstanceGroupManagerScopedListIterator) Next() (compute.InstanceGro i.internalCounter++ return pair, nil } + +type stubManagedInstanceIterator struct { + instances []*computepb.ManagedInstance + nextErr error + + internalCounter int +} + +func (i *stubManagedInstanceIterator) Next() (*computepb.ManagedInstance, error) { + if i.nextErr != nil { + return nil, i.nextErr + } + if i.internalCounter >= len(i.instances) { + return nil, iterator.Done + } + instance := i.instances[i.internalCounter] + i.internalCounter++ + return instance, nil +} diff --git a/operators/constellation-node-operator/internal/gcp/client/gcpwrappers.go b/operators/constellation-node-operator/internal/gcp/client/gcpwrappers.go index b00eaf2a4..7278f1644 100644 --- a/operators/constellation-node-operator/internal/gcp/client/gcpwrappers.go +++ b/operators/constellation-node-operator/internal/gcp/client/gcpwrappers.go @@ -42,32 +42,52 @@ func (c *instanceGroupManagersClient) Close() error { return c.InstanceGroupManagersClient.Close() } -func (c *instanceGroupManagersClient) Get(ctx context.Context, req *computepb.GetInstanceGroupManagerRequest, - opts ...gax.CallOption, -) (*computepb.InstanceGroupManager, error) { - return c.InstanceGroupManagersClient.Get(ctx, req, opts...) -} - -func (c *instanceGroupManagersClient) AggregatedList(ctx context.Context, req *computepb.AggregatedListInstanceGroupManagersRequest, +func (c *instanceGroupManagersClient) AggregatedList(ctx context.Context, + req *computepb.AggregatedListInstanceGroupManagersRequest, opts ...gax.CallOption, ) InstanceGroupManagerScopedListIterator { return c.InstanceGroupManagersClient.AggregatedList(ctx, req, opts...) } -func (c *instanceGroupManagersClient) SetInstanceTemplate(ctx context.Context, req *computepb.SetInstanceTemplateInstanceGroupManagerRequest, - opts ...gax.CallOption, -) (Operation, error) { - return c.InstanceGroupManagersClient.SetInstanceTemplate(ctx, req, opts...) +type regionInstanceGroupManagersClient struct { + *compute.RegionInstanceGroupManagersClient } -func (c *instanceGroupManagersClient) CreateInstances(ctx context.Context, req *computepb.CreateInstancesInstanceGroupManagerRequest, - opts ...gax.CallOption, -) (Operation, error) { - return c.InstanceGroupManagersClient.CreateInstances(ctx, req, opts...) +func (c *regionInstanceGroupManagersClient) Close() error { + return c.RegionInstanceGroupManagersClient.Close() } -func (c *instanceGroupManagersClient) DeleteInstances(ctx context.Context, req *computepb.DeleteInstancesInstanceGroupManagerRequest, +func (c *regionInstanceGroupManagersClient) Get(ctx context.Context, + req *computepb.GetRegionInstanceGroupManagerRequest, + opts ...gax.CallOption, +) (*computepb.InstanceGroupManager, error) { + return c.RegionInstanceGroupManagersClient.Get(ctx, req, opts...) +} + +func (c *regionInstanceGroupManagersClient) ListManagedInstances(ctx context.Context, + req *computepb.ListManagedInstancesRegionInstanceGroupManagersRequest, + opts ...gax.CallOption, +) ManagedInstanceIterator { + return c.RegionInstanceGroupManagersClient.ListManagedInstances(ctx, req, opts...) +} + +func (c *regionInstanceGroupManagersClient) SetInstanceTemplate(ctx context.Context, + req *computepb.SetInstanceTemplateRegionInstanceGroupManagerRequest, opts ...gax.CallOption, ) (Operation, error) { - return c.InstanceGroupManagersClient.DeleteInstances(ctx, req, opts...) + return c.RegionInstanceGroupManagersClient.SetInstanceTemplate(ctx, req, opts...) +} + +func (c *regionInstanceGroupManagersClient) CreateInstances(ctx context.Context, + req *computepb.CreateInstancesRegionInstanceGroupManagerRequest, + opts ...gax.CallOption, +) (Operation, error) { + return c.RegionInstanceGroupManagersClient.CreateInstances(ctx, req, opts...) +} + +func (c *regionInstanceGroupManagersClient) DeleteInstances(ctx context.Context, + req *computepb.DeleteInstancesRegionInstanceGroupManagerRequest, + opts ...gax.CallOption, +) (Operation, error) { + return c.RegionInstanceGroupManagersClient.DeleteInstances(ctx, req, opts...) } diff --git a/operators/constellation-node-operator/internal/gcp/client/instancegroupmanagers.go b/operators/constellation-node-operator/internal/gcp/client/instancegroupmanagers.go index fa81bd58c..bcafbdc89 100644 --- a/operators/constellation-node-operator/internal/gcp/client/instancegroupmanagers.go +++ b/operators/constellation-node-operator/internal/gcp/client/instancegroupmanagers.go @@ -13,10 +13,13 @@ import ( "strings" ) -var instanceGroupIDRegex = regexp.MustCompile(`^projects/([^/]+)/zones/([^/]+)/instanceGroupManagers/([^/]+)$`) +var ( + instanceGroupIDRegex = regexp.MustCompile(`^projects/([^/]+)/regions/([^/]+)/instanceGroupManagers/([^/]+)$`) + instanceIDRegex = regexp.MustCompile(`^projects/([^/]+)/zones/([^/]+)/instances/([^/]+)$`) +) func (c *Client) canonicalInstanceGroupID(ctx context.Context, instanceGroupID string) (string, error) { - project, zone, instanceGroup, err := splitInstanceGroupID(uriNormalize(instanceGroupID)) + project, region, instanceGroup, err := splitInstanceGroupID(uriNormalize(instanceGroupID)) if err != nil { return "", err } @@ -24,11 +27,11 @@ func (c *Client) canonicalInstanceGroupID(ctx context.Context, instanceGroupID s if err != nil { return "", err } - return fmt.Sprintf("projects/%s/zones/%s/instanceGroupManagers/%s", project, zone, instanceGroup), nil + return fmt.Sprintf("projects/%s/regions/%s/instanceGroupManagers/%s", project, region, instanceGroup), nil } // splitInstanceGroupID splits an instance group ID into core components. -func splitInstanceGroupID(instanceGroupID string) (project, zone, instanceGroup string, err error) { +func splitInstanceGroupID(instanceGroupID string) (project, region, instanceGroup string, err error) { matches := instanceGroupIDRegex.FindStringSubmatch(instanceGroupID) if len(matches) != 4 { return "", "", "", fmt.Errorf("error splitting instanceGroupID: %v", instanceGroupID) @@ -36,6 +39,15 @@ func splitInstanceGroupID(instanceGroupID string) (project, zone, instanceGroup return matches[1], matches[2], matches[3], nil } +// splitInstanceID splits an instance ID into core components. +func splitInstanceID(instanceID string) (project, zone, instanceName string, err error) { + matches := instanceIDRegex.FindStringSubmatch(instanceID) + if len(matches) != 4 { + return "", "", "", fmt.Errorf("error splitting instanceID: %v", instanceID) + } + return matches[1], matches[2], matches[3], nil +} + // isControlPlaneInstanceGroup returns true if the instance group is a control plane instance group. func isControlPlaneInstanceGroup(instanceGroupName string) bool { return strings.Contains(instanceGroupName, "control-plane") diff --git a/operators/constellation-node-operator/internal/gcp/client/instancegroupmanagers_test.go b/operators/constellation-node-operator/internal/gcp/client/instancegroupmanagers_test.go index e3523a60f..ad8877a78 100644 --- a/operators/constellation-node-operator/internal/gcp/client/instancegroupmanagers_test.go +++ b/operators/constellation-node-operator/internal/gcp/client/instancegroupmanagers_test.go @@ -21,14 +21,14 @@ func TestSplitInstanceGroupID(t *testing.T) { instanceGroupID string wantProject string - wantZone string + wantRegion string wantInstanceGroup string wantErr bool }{ "valid request": { - instanceGroupID: "projects/project/zones/zone/instanceGroupManagers/instanceGroup", + instanceGroupID: "projects/project/regions/region/instanceGroupManagers/instanceGroup", wantProject: "project", - wantZone: "zone", + wantRegion: "region", wantInstanceGroup: "instanceGroup", }, "wrong format": { @@ -40,19 +40,57 @@ func TestSplitInstanceGroupID(t *testing.T) { t.Run(name, func(t *testing.T) { assert := assert.New(t) require := require.New(t) - gotProject, gotZone, gotInstanceGroup, err := splitInstanceGroupID(tc.instanceGroupID) + gotProject, gotRegion, gotInstanceGroup, err := splitInstanceGroupID(tc.instanceGroupID) if tc.wantErr { assert.Error(err) return } require.NoError(err) assert.Equal(tc.wantProject, gotProject) - assert.Equal(tc.wantZone, gotZone) + assert.Equal(tc.wantRegion, gotRegion) assert.Equal(tc.wantInstanceGroup, gotInstanceGroup) }) } } +func TestSplitInstanceID(t *testing.T) { + testCases := map[string]struct { + instanceID string + wantProject string + wantZone string + wantInstanceName string + wantErr bool + }{ + "valid request": { + instanceID: "projects/project/zones/zone/instances/name", + wantProject: "project", + wantZone: "zone", + wantInstanceName: "name", + }, + "wrong format": { + instanceID: "wrong-format", + wantErr: true, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + assert := assert.New(t) + + gotProject, gotZone, gotInstanceName, err := splitInstanceID(tc.instanceID) + + if tc.wantErr { + assert.Error(err) + return + } + assert.NoError(err) + assert.Equal(tc.wantProject, gotProject) + assert.Equal(tc.wantZone, gotZone) + assert.Equal(tc.wantInstanceName, gotInstanceName) + }) + } +} + func TestGenerateInstanceName(t *testing.T) { assert := assert.New(t) baseInstanceName := "base" diff --git a/operators/constellation-node-operator/internal/gcp/client/nodeimage.go b/operators/constellation-node-operator/internal/gcp/client/nodeimage.go index 3b1d4d9a0..e333d1cba 100644 --- a/operators/constellation-node-operator/internal/gcp/client/nodeimage.go +++ b/operators/constellation-node-operator/internal/gcp/client/nodeimage.go @@ -8,8 +8,10 @@ package client import ( "context" + "errors" "fmt" + "google.golang.org/api/iterator" computepb "google.golang.org/genproto/googleapis/cloud/compute/v1" "google.golang.org/protobuf/proto" ) @@ -80,7 +82,7 @@ func (c *Client) GetScalingGroupID(ctx context.Context, providerID string) (stri // CreateNode creates a node in the specified scaling group. func (c *Client) CreateNode(ctx context.Context, scalingGroupID string) (nodeName, providerID string, err error) { - project, zone, instanceGroupName, err := splitInstanceGroupID(scalingGroupID) + project, region, instanceGroupName, err := splitInstanceGroupID(scalingGroupID) if err != nil { return "", "", err } @@ -88,10 +90,10 @@ func (c *Client) CreateNode(ctx context.Context, scalingGroupID string) (nodeNam if err != nil { return "", "", err } - instanceGroupManager, err := c.instanceGroupManagersAPI.Get(ctx, &computepb.GetInstanceGroupManagerRequest{ + instanceGroupManager, err := c.regionInstanceGroupManagersAPI.Get(ctx, &computepb.GetRegionInstanceGroupManagerRequest{ InstanceGroupManager: instanceGroupName, Project: project, - Zone: zone, + Region: region, }) if err != nil { return "", "", err @@ -100,11 +102,11 @@ func (c *Client) CreateNode(ctx context.Context, scalingGroupID string) (nodeNam return "", "", fmt.Errorf("instance group manager %q has no base instance name", instanceGroupName) } instanceName := generateInstanceName(*instanceGroupManager.BaseInstanceName, c.prng) - op, err := c.instanceGroupManagersAPI.CreateInstances(ctx, &computepb.CreateInstancesInstanceGroupManagerRequest{ + op, err := c.regionInstanceGroupManagersAPI.CreateInstances(ctx, &computepb.CreateInstancesRegionInstanceGroupManagerRequest{ InstanceGroupManager: instanceGroupName, Project: project, - Zone: zone, - InstanceGroupManagersCreateInstancesRequestResource: &computepb.InstanceGroupManagersCreateInstancesRequest{ + Region: region, + RegionInstanceGroupManagersCreateInstancesRequestResource: &computepb.RegionInstanceGroupManagersCreateInstancesRequest{ Instances: []*computepb.PerInstanceConfig{ {Name: proto.String(instanceName)}, }, @@ -116,6 +118,31 @@ func (c *Client) CreateNode(ctx context.Context, scalingGroupID string) (nodeNam if err := op.Wait(ctx); err != nil { return "", "", err } + + managedInstanceIter := c.regionInstanceGroupManagersAPI.ListManagedInstances(ctx, + &computepb.ListManagedInstancesRegionInstanceGroupManagersRequest{ + InstanceGroupManager: instanceGroupName, + Project: project, + Region: region, + Filter: proto.String(fmt.Sprintf("instance eq '.*%s'", instanceName)), + }, + ) + managedInstance, err := managedInstanceIter.Next() + if err != nil { + return "", "", fmt.Errorf("getting managed instance %q: %w", instanceName, err) + } + if _, err := managedInstanceIter.Next(); err != iterator.Done { + return "", "", fmt.Errorf("expected 1 managed instance with name %q but found multiple", instanceName) + } + if managedInstance.Instance == nil { + return "", "", errors.New("ListManagedInstances returned managedInstance with empty instance field") + } + + _, zone, _, err := splitInstanceID(uriNormalize(*managedInstance.Instance)) + if err != nil { + return "", "", fmt.Errorf("parsing zone of managed instance %q: %w", instanceName, err) + } + return instanceName, joinProviderID(project, zone, instanceName), nil } @@ -129,16 +156,16 @@ func (c *Client) DeleteNode(ctx context.Context, providerID string) error { if err != nil { return err } - instanceGroupProject, instanceGroupZone, instanceGroupName, err := splitInstanceGroupID(scalingGroupID) + instanceGroupProject, instanceGroupRegion, instanceGroupName, err := splitInstanceGroupID(scalingGroupID) if err != nil { return err } instanceID := joinInstanceID(zone, instanceName) - op, err := c.instanceGroupManagersAPI.DeleteInstances(ctx, &computepb.DeleteInstancesInstanceGroupManagerRequest{ + op, err := c.regionInstanceGroupManagersAPI.DeleteInstances(ctx, &computepb.DeleteInstancesRegionInstanceGroupManagerRequest{ InstanceGroupManager: instanceGroupName, Project: instanceGroupProject, - Zone: instanceGroupZone, - InstanceGroupManagersDeleteInstancesRequestResource: &computepb.InstanceGroupManagersDeleteInstancesRequest{ + Region: instanceGroupRegion, + RegionInstanceGroupManagersDeleteInstancesRequestResource: &computepb.RegionInstanceGroupManagersDeleteInstancesRequest{ Instances: []string{instanceID}, }, }) diff --git a/operators/constellation-node-operator/internal/gcp/client/nodeimage_test.go b/operators/constellation-node-operator/internal/gcp/client/nodeimage_test.go index a04306704..3ef20e4ae 100644 --- a/operators/constellation-node-operator/internal/gcp/client/nodeimage_test.go +++ b/operators/constellation-node-operator/internal/gcp/client/nodeimage_test.go @@ -122,8 +122,8 @@ func TestGetScalingGroupID(t *testing.T) { }{ "scaling group is found": { providerID: "gce://project/zone/instance-name", - createdBy: "projects/project/zones/zone/instanceGroupManagers/instance-group", - wantScalingGroupID: "projects/project/zones/zone/instanceGroupManagers/instance-group", + createdBy: "projects/project/regions/region/instanceGroupManagers/instance-group", + wantScalingGroupID: "projects/project/regions/region/instanceGroupManagers/instance-group", }, "splitting providerID fails": { providerID: "invalid", @@ -177,12 +177,13 @@ func TestCreateNode(t *testing.T) { testCases := map[string]struct { scalingGroupID string baseInstanceName *string + createdInstance *string getInstanceGroupManagerErr error createInstanceErr error wantErr bool }{ "scaling group is found": { - scalingGroupID: "projects/project/zones/zone/instanceGroupManagers/instance-group", + scalingGroupID: "projects/project/regions/region/instanceGroupManagers/instance-group", baseInstanceName: proto.String("base-name"), }, "splitting scalingGroupID fails": { @@ -190,16 +191,16 @@ func TestCreateNode(t *testing.T) { wantErr: true, }, "get instance group manager fails": { - scalingGroupID: "projects/project/zones/zone/instanceGroupManagers/instance-group", + scalingGroupID: "projects/project/regions/region/instanceGroupManagers/instance-group", getInstanceGroupManagerErr: errors.New("get instance group manager error"), wantErr: true, }, "instance group manager has no base instance name": { - scalingGroupID: "projects/project/zones/zone/instanceGroupManagers/instance-group", + scalingGroupID: "projects/project/regions/region/instanceGroupManagers/instance-group", wantErr: true, }, "create instance fails": { - scalingGroupID: "projects/project/zones/zone/instanceGroupManagers/instance-group", + scalingGroupID: "projects/project/regions/region/instanceGroupManagers/instance-group", baseInstanceName: proto.String("base-name"), createInstanceErr: errors.New("create instance error"), wantErr: true, @@ -212,12 +213,15 @@ func TestCreateNode(t *testing.T) { require := require.New(t) client := Client{ - instanceGroupManagersAPI: &stubInstanceGroupManagersAPI{ + regionInstanceGroupManagersAPI: &stubRegionInstanceGroupManagersAPI{ getErr: tc.getInstanceGroupManagerErr, createInstancesErr: tc.createInstanceErr, instanceGroupManager: &computepb.InstanceGroupManager{ BaseInstanceName: tc.baseInstanceName, }, + managedInstance: &computepb.ManagedInstance{ + Instance: tc.createdInstance, + }, }, prng: rand.New(rand.NewSource(int64(time.Now().Nanosecond()))), } @@ -236,14 +240,14 @@ func TestCreateNode(t *testing.T) { func TestDeleteNode(t *testing.T) { testCases := map[string]struct { providerID string - scalingGroupID string + instanceGroupID string getInstanceErr error deleteInstanceErr error wantErr bool }{ "node is deleted": { - providerID: "gce://project/zone/instance-name", - scalingGroupID: "projects/project/zones/zone/instanceGroupManagers/instance-group", + providerID: "gce://project/zone/instance-name", + instanceGroupID: "projects/project/regions/region/instanceGroupManagers/instance-group", }, "splitting providerID fails": { providerID: "invalid", @@ -254,14 +258,14 @@ func TestDeleteNode(t *testing.T) { getInstanceErr: errors.New("get instance error"), wantErr: true, }, - "splitting scalingGroupID fails": { - providerID: "gce://project/zone/instance-name", - scalingGroupID: "invalid", - wantErr: true, + "splitting instanceGroupID fails": { + providerID: "gce://project/zone/instance-name", + instanceGroupID: "invalid", + wantErr: true, }, "delete instance fails": { providerID: "gce://project/zone/instance-name", - scalingGroupID: "projects/project/zones/zone/instanceGroupManagers/instance-group", + instanceGroupID: "projects/project/regions/region/instanceGroupManagers/instance-group", deleteInstanceErr: errors.New("delete instance error"), wantErr: true, }, @@ -273,7 +277,7 @@ func TestDeleteNode(t *testing.T) { require := require.New(t) client := Client{ - instanceGroupManagersAPI: &stubInstanceGroupManagersAPI{ + regionInstanceGroupManagersAPI: &stubRegionInstanceGroupManagersAPI{ deleteInstancesErr: tc.deleteInstanceErr, }, instanceAPI: &stubInstanceAPI{ @@ -281,7 +285,7 @@ func TestDeleteNode(t *testing.T) { instance: &computepb.Instance{ Metadata: &computepb.Metadata{ Items: []*computepb.Items{ - {Key: proto.String("created-by"), Value: &tc.scalingGroupID}, + {Key: proto.String("created-by"), Value: &tc.instanceGroupID}, }, }, }, diff --git a/operators/constellation-node-operator/internal/gcp/client/scalinggroup.go b/operators/constellation-node-operator/internal/gcp/client/scalinggroup.go index 73bb1433f..09cc1de08 100644 --- a/operators/constellation-node-operator/internal/gcp/client/scalinggroup.go +++ b/operators/constellation-node-operator/internal/gcp/client/scalinggroup.go @@ -27,7 +27,7 @@ func (c *Client) GetScalingGroupImage(ctx context.Context, scalingGroupID string // SetScalingGroupImage sets the image URI of the scaling group. func (c *Client) SetScalingGroupImage(ctx context.Context, scalingGroupID, imageURI string) error { - project, zone, instanceGroupName, err := splitInstanceGroupID(scalingGroupID) + project, region, instanceGroupName, err := splitInstanceGroupID(scalingGroupID) if err != nil { return err } @@ -68,11 +68,11 @@ func (c *Client) SetScalingGroupImage(ctx context.Context, scalingGroupID, image newTemplateURI := joinInstanceTemplateURI(project, newTemplateName) // update instance group manager to use new template - op, err = c.instanceGroupManagersAPI.SetInstanceTemplate(ctx, &computepb.SetInstanceTemplateInstanceGroupManagerRequest{ + op, err = c.regionInstanceGroupManagersAPI.SetInstanceTemplate(ctx, &computepb.SetInstanceTemplateRegionInstanceGroupManagerRequest{ InstanceGroupManager: instanceGroupName, Project: project, - Zone: zone, - InstanceGroupManagersSetInstanceTemplateRequestResource: &computepb.InstanceGroupManagersSetInstanceTemplateRequest{ + Region: region, + RegionInstanceGroupManagersSetTemplateRequestResource: &computepb.RegionInstanceGroupManagersSetTemplateRequest{ InstanceTemplate: &newTemplateURI, }, }) @@ -98,11 +98,11 @@ func (c *Client) GetScalingGroupName(scalingGroupID string) (string, error) { // GetScalingGroupName retrieves the name of a scaling group as needed by the cluster-autoscaler. func (c *Client) GetAutoscalingGroupName(scalingGroupID string) (string, error) { - project, zone, instanceGroupName, err := splitInstanceGroupID(scalingGroupID) + project, region, instanceGroupName, err := splitInstanceGroupID(scalingGroupID) if err != nil { return "", fmt.Errorf("getting autoscaling scaling group name: %w", err) } - return ensureURIPrefixed(fmt.Sprintf("projects/%s/zones/%s/instanceGroups/%s", project, zone, instanceGroupName)), nil + return ensureURIPrefixed(fmt.Sprintf("projects/%s/regions/%s/instanceGroups/%s", project, region, instanceGroupName)), nil } // ListScalingGroups retrieves a list of scaling groups for the cluster. @@ -141,14 +141,14 @@ func (c *Client) ListScalingGroups(ctx context.Context, uid string) (controlPlan } func (c *Client) getScalingGroupTemplate(ctx context.Context, scalingGroupID string) (*computepb.InstanceTemplate, error) { - project, zone, instanceGroupName, err := splitInstanceGroupID(scalingGroupID) + project, region, instanceGroupName, err := splitInstanceGroupID(scalingGroupID) if err != nil { return nil, err } - instanceGroupManager, err := c.instanceGroupManagersAPI.Get(ctx, &computepb.GetInstanceGroupManagerRequest{ + instanceGroupManager, err := c.regionInstanceGroupManagersAPI.Get(ctx, &computepb.GetRegionInstanceGroupManagerRequest{ InstanceGroupManager: instanceGroupName, Project: project, - Zone: zone, + Region: region, }) if err != nil { return nil, fmt.Errorf("getting instance group manager %q: %w", instanceGroupName, err) diff --git a/operators/constellation-node-operator/internal/gcp/client/scalinggroup_test.go b/operators/constellation-node-operator/internal/gcp/client/scalinggroup_test.go index 810ba1418..1a32f9526 100644 --- a/operators/constellation-node-operator/internal/gcp/client/scalinggroup_test.go +++ b/operators/constellation-node-operator/internal/gcp/client/scalinggroup_test.go @@ -28,7 +28,7 @@ func TestGetScalingGroupImage(t *testing.T) { wantErr bool }{ "getting image works": { - scalingGroupID: "projects/project/zones/zone/instanceGroupManagers/instance-group", + scalingGroupID: "projects/project/regions/region/instanceGroupManagers/instance-group", instanceGroupManagerTemplateID: proto.String("projects/project/global/instanceTemplates/instance-template"), instanceTemplate: &computepb.InstanceTemplate{ Properties: &computepb.InstanceProperties{ @@ -48,27 +48,27 @@ func TestGetScalingGroupImage(t *testing.T) { wantErr: true, }, "get instance fails": { - scalingGroupID: "projects/project/zones/zone/instanceGroupManagers/instance-group", + scalingGroupID: "projects/project/regions/region/instanceGroupManagers/instance-group", getInstanceGroupManagerErr: errors.New("get instance error"), wantErr: true, }, "instance group manager has no template": { - scalingGroupID: "projects/project/zones/zone/instanceGroupManagers/instance-group", + scalingGroupID: "projects/project/regions/region/instanceGroupManagers/instance-group", wantErr: true, }, "instance group manager template id is invalid": { - scalingGroupID: "projects/project/zones/zone/instanceGroupManagers/instance-group", + scalingGroupID: "projects/project/regions/region/instanceGroupManagers/instance-group", instanceGroupManagerTemplateID: proto.String("invalid"), wantErr: true, }, "get instance template fails": { - scalingGroupID: "projects/project/zones/zone/instanceGroupManagers/instance-group", + scalingGroupID: "projects/project/regions/region/instanceGroupManagers/instance-group", instanceGroupManagerTemplateID: proto.String("projects/project/global/instanceTemplates/instance-template"), getInstanceTemplateErr: errors.New("get instance template error"), wantErr: true, }, "instance template has no disks": { - scalingGroupID: "projects/project/zones/zone/instanceGroupManagers/instance-group", + scalingGroupID: "projects/project/regions/region/instanceGroupManagers/instance-group", instanceGroupManagerTemplateID: proto.String("projects/project/global/instanceTemplates/instance-template"), instanceTemplate: &computepb.InstanceTemplate{ Properties: &computepb.InstanceProperties{}, @@ -83,7 +83,7 @@ func TestGetScalingGroupImage(t *testing.T) { require := require.New(t) client := Client{ - instanceGroupManagersAPI: &stubInstanceGroupManagersAPI{ + regionInstanceGroupManagersAPI: &stubRegionInstanceGroupManagersAPI{ getErr: tc.getInstanceGroupManagerErr, instanceGroupManager: &computepb.InstanceGroupManager{ InstanceTemplate: tc.instanceGroupManagerTemplateID, @@ -118,7 +118,7 @@ func TestSetScalingGroupImage(t *testing.T) { wantErr bool }{ "setting image works": { - scalingGroupID: "projects/project/zones/zone/instanceGroupManagers/instance-group", + scalingGroupID: "projects/project/regions/region/instanceGroupManagers/instance-group", imageURI: "projects/project/global/images/image-2", instanceGroupManagerTemplateID: proto.String("projects/project/global/instanceTemplates/instance-template"), instanceTemplate: &computepb.InstanceTemplate{ @@ -135,7 +135,7 @@ func TestSetScalingGroupImage(t *testing.T) { }, }, "same image already in use": { - scalingGroupID: "projects/project/zones/zone/instanceGroupManagers/instance-group", + scalingGroupID: "projects/project/regions/region/instanceGroupManagers/instance-group", imageURI: "projects/project/global/images/image", instanceGroupManagerTemplateID: proto.String("projects/project/global/instanceTemplates/instance-template"), instanceTemplate: &computepb.InstanceTemplate{ @@ -158,27 +158,27 @@ func TestSetScalingGroupImage(t *testing.T) { wantErr: true, }, "get instance fails": { - scalingGroupID: "projects/project/zones/zone/instanceGroupManagers/instance-group", + scalingGroupID: "projects/project/regions/region/instanceGroupManagers/instance-group", getInstanceGroupManagerErr: errors.New("get instance error"), wantErr: true, }, "instance group manager has no template": { - scalingGroupID: "projects/project/zones/zone/instanceGroupManagers/instance-group", + scalingGroupID: "projects/project/regions/region/instanceGroupManagers/instance-group", wantErr: true, }, "instance group manager template id is invalid": { - scalingGroupID: "projects/project/zones/zone/instanceGroupManagers/instance-group", + scalingGroupID: "projects/project/regions/region/instanceGroupManagers/instance-group", instanceGroupManagerTemplateID: proto.String("invalid"), wantErr: true, }, "get instance template fails": { - scalingGroupID: "projects/project/zones/zone/instanceGroupManagers/instance-group", + scalingGroupID: "projects/project/regions/region/instanceGroupManagers/instance-group", instanceGroupManagerTemplateID: proto.String("projects/project/global/instanceTemplates/instance-template"), getInstanceTemplateErr: errors.New("get instance template error"), wantErr: true, }, "instance template has no disks": { - scalingGroupID: "projects/project/zones/zone/instanceGroupManagers/instance-group", + scalingGroupID: "projects/project/regions/region/instanceGroupManagers/instance-group", instanceGroupManagerTemplateID: proto.String("projects/project/global/instanceTemplates/instance-template"), instanceTemplate: &computepb.InstanceTemplate{ Properties: &computepb.InstanceProperties{}, @@ -186,7 +186,7 @@ func TestSetScalingGroupImage(t *testing.T) { wantErr: true, }, "instance template has no name": { - scalingGroupID: "projects/project/zones/zone/instanceGroupManagers/instance-group", + scalingGroupID: "projects/project/regions/region/instanceGroupManagers/instance-group", imageURI: "projects/project/global/images/image-2", instanceGroupManagerTemplateID: proto.String("projects/project/global/instanceTemplates/instance-template"), instanceTemplate: &computepb.InstanceTemplate{ @@ -203,7 +203,7 @@ func TestSetScalingGroupImage(t *testing.T) { wantErr: true, }, "instance template name generation fails": { - scalingGroupID: "projects/project/zones/zone/instanceGroupManagers/instance-group", + scalingGroupID: "projects/project/regions/region/instanceGroupManagers/instance-group", imageURI: "projects/project/global/images/image-2", instanceGroupManagerTemplateID: proto.String("projects/project/global/instanceTemplates/instance-template"), instanceTemplate: &computepb.InstanceTemplate{ @@ -221,7 +221,7 @@ func TestSetScalingGroupImage(t *testing.T) { wantErr: true, }, "instance template insert fails": { - scalingGroupID: "projects/project/zones/zone/instanceGroupManagers/instance-group", + scalingGroupID: "projects/project/regions/region/instanceGroupManagers/instance-group", imageURI: "projects/project/global/images/image-2", instanceGroupManagerTemplateID: proto.String("projects/project/global/instanceTemplates/instance-template"), instanceTemplate: &computepb.InstanceTemplate{ @@ -240,7 +240,7 @@ func TestSetScalingGroupImage(t *testing.T) { wantErr: true, }, "setting instance template fails": { - scalingGroupID: "projects/project/zones/zone/instanceGroupManagers/instance-group", + scalingGroupID: "projects/project/regions/region/instanceGroupManagers/instance-group", imageURI: "projects/project/global/images/image-2", instanceGroupManagerTemplateID: proto.String("projects/project/global/instanceTemplates/instance-template"), instanceTemplate: &computepb.InstanceTemplate{ @@ -266,7 +266,7 @@ func TestSetScalingGroupImage(t *testing.T) { require := require.New(t) client := Client{ - instanceGroupManagersAPI: &stubInstanceGroupManagersAPI{ + regionInstanceGroupManagersAPI: &stubRegionInstanceGroupManagersAPI{ getErr: tc.getInstanceGroupManagerErr, setInstanceTemplateErr: tc.setInstanceTemplateErr, instanceGroupManager: &computepb.InstanceGroupManager{ @@ -296,7 +296,7 @@ func TestGetScalingGroupName(t *testing.T) { wantErr bool }{ "valid scaling group ID": { - scalingGroupID: "projects/project/zones/zone/instanceGroupManagers/instance-group", + scalingGroupID: "projects/project/regions/region/instanceGroupManagers/instance-group", wantName: "instance-group", }, "invalid scaling group ID": { @@ -337,21 +337,21 @@ func TestListScalingGroups(t *testing.T) { }, "list instance group managers for control plane": { name: proto.String("test-control-plane-uid"), - groupID: proto.String("projects/project/zones/zone/instanceGroupManagers/test-control-plane-uid"), + groupID: proto.String("projects/project/regions/region/instanceGroupManagers/test-control-plane-uid"), wantControlPlanes: []string{ - "projects/project/zones/zone/instanceGroupManagers/test-control-plane-uid", + "projects/project/regions/region/instanceGroupManagers/test-control-plane-uid", }, }, "list instance group managers for worker": { name: proto.String("test-worker-uid"), - groupID: proto.String("projects/project/zones/zone/instanceGroupManagers/test-worker-uid"), + groupID: proto.String("projects/project/regions/region/instanceGroupManagers/test-worker-uid"), wantWorkers: []string{ - "projects/project/zones/zone/instanceGroupManagers/test-worker-uid", + "projects/project/regions/region/instanceGroupManagers/test-worker-uid", }, }, "unrelated instance group manager": { name: proto.String("test-unrelated-uid"), - groupID: proto.String("projects/project/zones/zone/instanceGroupManagers/test-unrelated-uid"), + groupID: proto.String("projects/project/regions/region/instanceGroupManagers/test-unrelated-uid"), }, "invalid instance group manager": {}, }