Skip to content

Commit

Permalink
Merge pull request #8347 from mtulio/CORS-3479-fix-control-plane-plac…
Browse files Browse the repository at this point in the history
…ement

CORS-2895: capa/machines: fix zone placement for control planes
  • Loading branch information
openshift-merge-bot[bot] authored May 10, 2024
2 parents 18b9aa2 + d56a003 commit a424a7f
Show file tree
Hide file tree
Showing 3 changed files with 282 additions and 13 deletions.
29 changes: 17 additions & 12 deletions pkg/asset/machines/aws/awsmachines.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,24 +46,29 @@ func GenerateMachines(clusterID string, in *MachineInput) ([]*asset.RuntimeFile,
var result []*asset.RuntimeFile

for idx := int64(0); idx < total; idx++ {
var subnet *capa.AWSResourceReference
// By not setting subnets for the machine, we let CAPA choose one for us
subnet := &capa.AWSResourceReference{}
zone := mpool.Zones[int(idx)%len(mpool.Zones)]

// BYO VPC deployments when subnet IDs are set on install-config.yaml
if len(in.Subnets) > 0 {
zone := mpool.Zones[int(idx)%len(mpool.Zones)]
subnetID, ok := in.Subnets[zone]
if len(in.Subnets) > 0 && !ok {
return nil, fmt.Errorf("no subnet for zone %s", zone)
}
subnet = &capa.AWSResourceReference{}
if subnetID == "" {
subnet.Filters = []capa.Filter{
{
Name: "tag:Name",
Values: []string{fmt.Sprintf("%s-subnet-private-%s", clusterID, zone)},
},
}
} else {
subnet.ID = ptr.To(subnetID)
return nil, fmt.Errorf("invalid subnet ID for zone %s", zone)
}
subnet.ID = ptr.To(subnetID)
} else {
subnetInternetScope := "private"
if in.PublicIP {
subnetInternetScope = "public"
}
subnet.Filters = []capa.Filter{
{
Name: "tag:Name",
Values: []string{fmt.Sprintf("%s-subnet-%s-%s", clusterID, subnetInternetScope, zone)},
},
}
}

Expand Down
250 changes: 250 additions & 0 deletions pkg/asset/machines/aws/awsmachines_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,250 @@
// Package aws generates Machine objects for aws.
package aws

import (
"fmt"
"strings"
"testing"

"github.com/google/go-cmp/cmp"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/utils/ptr"
capa "sigs.k8s.io/cluster-api-provider-aws/v2/api/v1beta2"

"github.com/openshift/installer/pkg/asset"
"github.com/openshift/installer/pkg/types"
awstypes "github.com/openshift/installer/pkg/types/aws"
)

var stubMachineInputManagedVpc = &MachineInput{
Role: "master",
Pool: &types.MachinePool{
Name: "master",
Replicas: ptr.To(int64(3)),
Platform: types.MachinePoolPlatform{
AWS: &awstypes.MachinePool{
Zones: []string{"A", "B", "C"},
},
},
},
Subnets: make(map[string]string, 0),
Tags: capa.Tags{},
PublicIP: false,
Ignition: &capa.Ignition{},
}

func stubDeepCopyMachineInput(in *MachineInput) *MachineInput {
out := &MachineInput{
Role: in.Role,
PublicIP: in.PublicIP,
}
if in.Pool != nil {
out.Pool = &types.MachinePool{}
*out.Pool = *in.Pool
}
if len(in.Subnets) > 0 {
out.Subnets = make(map[string]string, len(in.Subnets))
for k, v := range in.Subnets {
out.Subnets[k] = v
}
}
if len(in.Tags) > 0 {
out.Tags = in.Tags.DeepCopy()
}
if in.Ignition != nil {
out.Ignition = in.Ignition.DeepCopy()
}
return out
}

func stubGetMachineManagedVpc() *MachineInput {
return stubDeepCopyMachineInput(stubMachineInputManagedVpc)
}

func TestGenerateMachines(t *testing.T) {
stubClusterID := "vpc-zr2-m2"
tests := []struct {
name string
clusterID string
input *MachineInput
want []*asset.RuntimeFile
wantInfraFiles []*asset.RuntimeFile
wantErr string
}{
{
name: "topology ha, managed vpc, default zones region, 2 zones A and B, 3 machines should be in A, B and A private subnet",
clusterID: stubClusterID,
input: func() *MachineInput {
in := stubGetMachineManagedVpc()
in.Pool.Platform.AWS.Zones = []string{"A", "B"}
return in
}(),
// generate 3 AWSMachine manifests for control plane nodes in two zones
wantInfraFiles: func() []*asset.RuntimeFile {
machineZoneMap := map[int]string{0: "A", 1: "B", 2: "A"}
infraMachineFiles := []*asset.RuntimeFile{}
for mid := 0; mid < 3; mid++ {
machineName := fmt.Sprintf("%s-%s-%d", stubClusterID, "master", mid)
machineZone := machineZoneMap[mid]
machine := &capa.AWSMachine{
TypeMeta: metav1.TypeMeta{
APIVersion: "infrastructure.cluster.x-k8s.io/v1beta2",
Kind: "AWSMachine",
},
ObjectMeta: metav1.ObjectMeta{
Name: machineName,
Labels: map[string]string{"cluster.x-k8s.io/control-plane": ""},
},
Spec: capa.AWSMachineSpec{
InstanceMetadataOptions: &capa.InstanceMetadataOptions{
HTTPEndpoint: capa.InstanceMetadataEndpointStateEnabled,
},
AMI: capa.AMIReference{
ID: ptr.To(""),
},
IAMInstanceProfile: fmt.Sprintf("%s-%s-profile", stubClusterID, "master"),
PublicIP: ptr.To(false),
Subnet: &capa.AWSResourceReference{
Filters: []capa.Filter{{Name: "tag:Name", Values: []string{
fmt.Sprintf("%s-subnet-private-%s", stubClusterID, machineZone),
}}},
},
SSHKeyName: ptr.To(""),
RootVolume: &capa.Volume{
Encrypted: ptr.To(true),
},
UncompressedUserData: ptr.To(true),
Ignition: &capa.Ignition{},
},
}
infraMachineFiles = append(infraMachineFiles, &asset.RuntimeFile{
File: asset.File{Filename: fmt.Sprintf("10_inframachine_%s.yaml", machineName)},
Object: machine,
})
}
return infraMachineFiles
}(),
},
{
name: "topology ha, byo vpc, two zones subnets A and B, 3 machines should be in A, B and A private subnets",
clusterID: stubClusterID,
input: func() *MachineInput {
in := stubGetMachineManagedVpc()
in.Pool.Platform.AWS.Zones = []string{"A", "B"}
in.Subnets = map[string]string{"A": "subnet-id-A", "B": "subnet-id-B"}
return in
}(),
// generate 3 AWSMachine manifests for control plane nodes in two subnets/zones
wantInfraFiles: func() []*asset.RuntimeFile {
machineZoneMap := map[int]string{0: "subnet-id-A", 1: "subnet-id-B", 2: "subnet-id-A"}
infraMachineFiles := []*asset.RuntimeFile{}
for mid := 0; mid < 3; mid++ {
machineName := fmt.Sprintf("%s-%s-%d", stubClusterID, "master", mid)
machineSubnet := machineZoneMap[mid]
machine := &capa.AWSMachine{
TypeMeta: metav1.TypeMeta{
APIVersion: "infrastructure.cluster.x-k8s.io/v1beta2",
Kind: "AWSMachine",
},
ObjectMeta: metav1.ObjectMeta{
Name: machineName,
Labels: map[string]string{"cluster.x-k8s.io/control-plane": ""},
},
Spec: capa.AWSMachineSpec{
InstanceMetadataOptions: &capa.InstanceMetadataOptions{
HTTPEndpoint: capa.InstanceMetadataEndpointStateEnabled,
},
AMI: capa.AMIReference{
ID: ptr.To(""),
},
IAMInstanceProfile: fmt.Sprintf("%s-%s-profile", stubClusterID, "master"),
PublicIP: ptr.To(false),
Subnet: &capa.AWSResourceReference{
ID: ptr.To(machineSubnet),
},
SSHKeyName: ptr.To(""),
RootVolume: &capa.Volume{
Encrypted: ptr.To(true),
},
UncompressedUserData: ptr.To(true),
Ignition: &capa.Ignition{},
},
}
infraMachineFiles = append(infraMachineFiles, &asset.RuntimeFile{
File: asset.File{Filename: fmt.Sprintf("10_inframachine_%s.yaml", machineName)},
Object: machine,
})
}
return infraMachineFiles
}(),
},
// Error's scenarios
{
name: "error topology ha, byo vpc, no subnet for zones",
clusterID: stubClusterID,
input: func() *MachineInput {
in := stubGetMachineManagedVpc()
in.Pool.Platform.AWS.Zones = []string{"A", "B"}
in.Subnets = map[string]string{"C": "subnet-id-C", "D": "subnet-id-D"}
return in
}(),
wantErr: `no subnet for zone A`,
},
{
name: "error topology ha, managed vpc, empty subnet zone",
clusterID: stubClusterID,
input: func() *MachineInput {
in := stubGetMachineManagedVpc()
in.Pool.Platform.AWS.Zones = []string{"A", "B"}
in.Subnets = map[string]string{"A": "subnet-id-A", "B": ""}
return in
}(),
wantErr: `invalid subnet ID for zone B`,
},
// TODO: add more use cases.
// {
// name: "managed vpc, default zones region, 5 zones A to E, 3 machines should be in A, B and C private subnet",
// },
// {
// name: "managed vpc, default zones region, 5 zones A to E, 3 machines should be in A, B and C private subnet",
// },
// {
// name: "byo vpc, 2 zones subnets A and B, 3 machines' subnet should be in A, B and A",
// },
// {
// name: "managed vpc, default zones region, 5 zones A to E, bootstrap should be in zone A public subnet",
// },
// {
// name: "topology ha, managed vpc, two zones subnets A and B, bootstrap node using public subnet A",
// },
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
files, err := GenerateMachines(tt.clusterID, tt.input)
if err != nil {
if len(tt.wantErr) > 0 {
if got := err.Error(); !cmp.Equal(got, tt.wantErr) {
t.Errorf("GenerateMachines() unexpected error message: %v", cmp.Diff(got, tt.wantErr))
}
return
}
t.Errorf("GenerateMachines() unexpected error: %v", err)
return
}
// TODO: support the CAPA v1beta1.Machine manifest check.
// Support only comparing manifest file for CAPA v1beta2.AWSMachine.
if len(tt.wantInfraFiles) > 0 {
got := []*asset.RuntimeFile{}
for _, file := range files {
if !strings.HasPrefix(file.Filename, "10_inframachine") {
continue
}
got = append(got, file)
}
if !cmp.Equal(got, tt.wantInfraFiles) {
t.Errorf("GenerateMachines() Got unexpected results:\n%v", cmp.Diff(got, tt.wantInfraFiles))
}
}
})
}
}
16 changes: 15 additions & 1 deletion pkg/asset/machines/clusterapi.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,14 +91,28 @@ func (c *ClusterAPI) Generate(dependencies asset.Parents) error {
switch ic.Platform.Name() {
case awstypes.Name:
subnets := map[string]string{}
bootstrapSubnets := map[string]string{}
if len(ic.Platform.AWS.Subnets) > 0 {
// fetch private subnets to master nodes.
subnetMeta, err := installConfig.AWS.PrivateSubnets(ctx)
if err != nil {
return err
}
for id, subnet := range subnetMeta {
subnets[subnet.Zone.Name] = id
}
// fetch public subnets for bootstrap, when exists, otherwise use private.
if installConfig.Config.Publish == types.ExternalPublishingStrategy {
subnetMeta, err := installConfig.AWS.PublicSubnets(ctx)
if err != nil {
return err
}
for id, subnet := range subnetMeta {
bootstrapSubnets[subnet.Zone.Name] = id
}
} else {
bootstrapSubnets = subnets
}
}

mpool := defaultAWSMachinePoolPlatform("master")
Expand Down Expand Up @@ -177,7 +191,7 @@ func (c *ClusterAPI) Generate(dependencies asset.Parents) error {
pool.Platform.AWS = &mpool
bootstrapAWSMachine, err := aws.GenerateMachines(clusterID.InfraID, &aws.MachineInput{
Role: "bootstrap",
Subnets: nil, // let CAPA pick one
Subnets: bootstrapSubnets,
Pool: &pool,
Tags: tags,
PublicIP: installConfig.Config.Publish == types.ExternalPublishingStrategy,
Expand Down

0 comments on commit a424a7f

Please sign in to comment.