Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unreleased AWS Placement Groups support #35

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

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
Expand Up @@ -114,7 +114,7 @@ test-e2e: ## Run e2e tests

.PHONY: lint
lint: ## Go lint your code
$(DOCKER_CMD) hack/go-lint.sh -min_confidence 0.3 $$(go list -f '{{ .ImportPath }}' ./... | grep -v -e 'github.com/openshift/machine-api-provider-aws/test' -e 'github.com/openshift/machine-api-provider-aws/pkg/cloud/aws/client/mock')
$(DOCKER_CMD) hack/go-lint.sh -min_confidence 0.3 $$(go list -f '{{ .ImportPath }}' ./... | grep -v -e 'github.com/openshift/machine-api-provider-aws/test' -e 'github.com/openshift/machine-api-provider-aws/pkg/cloud/aws/client/mock' -e 'github.com/openshift/machine-api-provider-aws/pkg/api/machine/v1')

.PHONY: fmt
fmt: ## Go fmt your code
Expand Down
9 changes: 0 additions & 9 deletions cmd/manager/main.go
Expand Up @@ -24,7 +24,6 @@ import (
machinev1beta1 "github.com/openshift/api/machine/v1beta1"
"github.com/openshift/machine-api-operator/pkg/controller/machine"
"github.com/openshift/machine-api-operator/pkg/metrics"
awsplacementgroupcontroller "github.com/openshift/machine-api-provider-aws/pkg/actuators/awsplacementgroup"
machineactuator "github.com/openshift/machine-api-provider-aws/pkg/actuators/machine"
machinesetcontroller "github.com/openshift/machine-api-provider-aws/pkg/actuators/machineset"
awsclient "github.com/openshift/machine-api-provider-aws/pkg/client"
Expand Down Expand Up @@ -179,14 +178,6 @@ func main() {
os.Exit(1)
}

if err = (&awsplacementgroupcontroller.Reconciler{
Client: mgr.GetClient(),
Log: ctrl.Log.WithName("controllers").WithName("AWSPlacementGroup"),
}).SetupWithManager(mgr, controller.Options{}); err != nil {
setupLog.Error(err, "unable to create controller", "controller", "AWSPlacementGroup")
os.Exit(1)
}

if err := mgr.AddReadyzCheck("ping", healthz.Ping); err != nil {
klog.Fatal(err)
}
Expand Down
4 changes: 2 additions & 2 deletions go.mod
Expand Up @@ -9,7 +9,7 @@ require (
github.com/golang/mock v1.5.0
github.com/onsi/ginkgo v1.16.5
github.com/onsi/gomega v1.17.0
github.com/openshift/api v0.0.0-20220412161459-8f38b7648620
github.com/openshift/api v0.0.0-20220429222041-b25f69a603a7
github.com/openshift/machine-api-operator v0.2.1-0.20220327131531-58ba8507d869
k8s.io/api v0.23.0
k8s.io/apimachinery v0.23.0
Expand Down Expand Up @@ -83,7 +83,7 @@ require (
go.starlark.net v0.0.0-20200306205701-8dd3e2ee1dd5 // indirect
golang.org/x/mod v0.4.2 // indirect
golang.org/x/net v0.0.0-20220127200216-cd36cc0744dd // indirect
golang.org/x/oauth2 v0.0.0-20210819190943-2bc19b11175f // indirect
golang.org/x/oauth2 v0.0.0-20220411215720-9780585627b5 // indirect
golang.org/x/sys v0.0.0-20211216021012-1d35b9e2eb4e // indirect
golang.org/x/term v0.0.0-20210927222741-03fcf44c2211 // indirect
golang.org/x/text v0.3.7 // indirect
Expand Down
7 changes: 4 additions & 3 deletions go.sum
Expand Up @@ -553,8 +553,8 @@ github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3I
github.com/opencontainers/image-spec v1.0.1/go.mod h1:BtxoFyWECRxE4U/7sNtV5W15zMzWCbyJoFRP3s7yZA0=
github.com/openshift/api v0.0.0-20211209135129-c58d9f695577/go.mod h1:DoslCwtqUpr3d/gsbq4ZlkaMEdYqKxuypsDjorcHhME=
github.com/openshift/api v0.0.0-20220322000322-9c4998a4d646/go.mod h1:F/eU6jgr6Q2VhMu1mSpMmygxAELd7+BUxs3NHZ25jV4=
github.com/openshift/api v0.0.0-20220412161459-8f38b7648620 h1:eNClZcEJo3EUIMCkK2GPrXl4lrFNfyKGRFVrUEZ8JIk=
github.com/openshift/api v0.0.0-20220412161459-8f38b7648620/go.mod h1:F/eU6jgr6Q2VhMu1mSpMmygxAELd7+BUxs3NHZ25jV4=
github.com/openshift/api v0.0.0-20220429222041-b25f69a603a7 h1:G9EAAWcAzv2+eWN2wlpDQJ4OFbgkmfAg4ppzvE3k2xs=
github.com/openshift/api v0.0.0-20220429222041-b25f69a603a7/go.mod h1:F/eU6jgr6Q2VhMu1mSpMmygxAELd7+BUxs3NHZ25jV4=
github.com/openshift/build-machinery-go v0.0.0-20210712174854-1bb7fd1518d3/go.mod h1:b1BuldmJlbA/xYtdZvKi+7j5YGB44qJUJDZ9zwiNCfE=
github.com/openshift/build-machinery-go v0.0.0-20211213093930-7e33a7eb4ce3/go.mod h1:b1BuldmJlbA/xYtdZvKi+7j5YGB44qJUJDZ9zwiNCfE=
github.com/openshift/client-go v0.0.0-20211209144617-7385dd6338e3 h1:SG1aqwleU6bGD0X4mhkTNupjVnByMYYuW4XbnCPavQU=
Expand Down Expand Up @@ -869,8 +869,9 @@ golang.org/x/oauth2 v0.0.0-20210220000619-9bb904979d93/go.mod h1:KelEdhl1UZF7XfJ
golang.org/x/oauth2 v0.0.0-20210313182246-cd4f82c27b84/go.mod h1:KelEdhl1UZF7XfJ4dDtk6s++YSgaE7mD/BuKKDLBl4A=
golang.org/x/oauth2 v0.0.0-20210402161424-2e8d93401602/go.mod h1:KelEdhl1UZF7XfJ4dDtk6s++YSgaE7mD/BuKKDLBl4A=
golang.org/x/oauth2 v0.0.0-20210514164344-f6687ab2804c/go.mod h1:KelEdhl1UZF7XfJ4dDtk6s++YSgaE7mD/BuKKDLBl4A=
golang.org/x/oauth2 v0.0.0-20210819190943-2bc19b11175f h1:Qmd2pbz05z7z6lm0DrgQVVPuBm92jqujBKMHMOlOQEw=
golang.org/x/oauth2 v0.0.0-20210819190943-2bc19b11175f/go.mod h1:KelEdhl1UZF7XfJ4dDtk6s++YSgaE7mD/BuKKDLBl4A=
golang.org/x/oauth2 v0.0.0-20220411215720-9780585627b5 h1:OSnWWcOd/CtWQC2cYSBgbTSJv3ciqd8r54ySIW2y3RE=
golang.org/x/oauth2 v0.0.0-20220411215720-9780585627b5/go.mod h1:DAh4E804XQdzx2j+YRIaUnCqCV2RuMz24cGBJ5QYIrc=
golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
Expand Down
2 changes: 1 addition & 1 deletion pkg/actuators/awsplacementgroup/controller.go
Expand Up @@ -5,7 +5,7 @@ import (
"fmt"

"github.com/go-logr/logr"
machinev1 "github.com/openshift/api/machine/v1"
machinev1 "github.com/openshift/machine-api-provider-aws/pkg/api/machine/v1"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
Expand Down
21 changes: 0 additions & 21 deletions pkg/actuators/machine/instances.go
@@ -1,7 +1,6 @@
package machine

import (
"context"
"encoding/base64"
"errors"
"fmt"
Expand All @@ -14,12 +13,10 @@ import (
"github.com/aws/aws-sdk-go/aws/awserr"
"github.com/aws/aws-sdk-go/service/ec2"
configv1 "github.com/openshift/api/config/v1"
machinev1 "github.com/openshift/api/machine/v1"
machinev1beta1 "github.com/openshift/api/machine/v1beta1"
mapierrors "github.com/openshift/machine-api-operator/pkg/controller/machine"
"github.com/openshift/machine-api-operator/pkg/metrics"
awsclient "github.com/openshift/machine-api-provider-aws/pkg/client"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/klog/v2"
runtimeclient "sigs.k8s.io/controller-runtime/pkg/client"
)
Expand Down Expand Up @@ -530,24 +527,6 @@ func constructInstancePlacement(machine *machinev1beta1.Machine, machineProvider
machinev1beta1.HostTenancy)
}

if machineProviderConfig.Placement.Group.Name != "" {
pgKey := runtimeclient.ObjectKey{Namespace: machine.GetNamespace(), Name: machineProviderConfig.Placement.Group.Name}
pg := &machinev1.AWSPlacementGroup{}
if err := client.Get(context.TODO(), pgKey, pg); apierrors.IsNotFound(err) {
return nil, mapierrors.InvalidMachineConfiguration("AWSPlacementGroup %q not found. Please create an AWSPlacementGroup before setting placement.group.name", machineProviderConfig.Placement.Group.Name)
} else if err != nil {
return nil, fmt.Errorf("could not check for placement group: %v", err)
}
placement.SetGroupName(machineProviderConfig.Placement.Group.Name)

if machineProviderConfig.Placement.PartitionNumber != 0 {
if pg.Spec.ManagementSpec.Managed != nil && pg.Spec.ManagementSpec.Managed.GroupType != machinev1.AWSPartitionPlacementGroupType {
return nil, mapierrors.InvalidMachineConfiguration("placement.partitionNumber may only be set when used with a AWSPlacementGroup with groupType \"Partition\"")
}
placement.SetPartitionNumber(int64(machineProviderConfig.Placement.PartitionNumber))
}
}

if *placement == (ec2.Placement{}) {
// If the placement is empty, we should just return a nil so as not to pollute the RunInstancesInput
return nil, nil
Expand Down
109 changes: 0 additions & 109 deletions pkg/actuators/machine/instances_test.go
Expand Up @@ -12,7 +12,6 @@ import (
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/golang/mock/gomock"
configv1 "github.com/openshift/api/config/v1"
machinev1 "github.com/openshift/api/machine/v1"
machinev1beta1 "github.com/openshift/api/machine/v1beta1"
mockaws "github.com/openshift/machine-api-provider-aws/pkg/client/mock"
"k8s.io/apimachinery/pkg/runtime"
Expand Down Expand Up @@ -971,114 +970,6 @@ func TestLaunchInstance(t *testing.T) {
UserData: aws.String(""),
},
},
{
name: "With a Placement Group Name which does not exist",
instancesOutput: stubReservation(stubAMIID, stubInstanceID, "192.168.0.10"),
providerConfig: stubPlacementGroupNameConfig(),
succeeds: false,
infra: infra,
},
{
name: "With a Placement Group Name which does exist",
instancesOutput: stubReservation(stubAMIID, stubInstanceID, "192.168.0.10"),
providerConfig: stubPlacementGroupNameConfig(),
succeeds: true,
infra: infra,
objects: []runtime.Object{
stubPlacementGroup(machinev1.AWSClusterPlacementGroupType),
},
runInstancesInput: &ec2.RunInstancesInput{
IamInstanceProfile: &ec2.IamInstanceProfileSpecification{
Name: aws.String(*providerConfig.IAMInstanceProfile.ID),
},
ImageId: aws.String(*providerConfig.AMI.ID),
InstanceType: &providerConfig.InstanceType,
MinCount: aws.Int64(1),
MaxCount: aws.Int64(1),
KeyName: providerConfig.KeyName,
TagSpecifications: []*ec2.TagSpecification{{
ResourceType: aws.String("instance"),
Tags: stubTagListWithInfraObject,
}, {
ResourceType: aws.String("volume"),
Tags: stubTagListWithInfraObject,
}},
NetworkInterfaces: []*ec2.InstanceNetworkInterfaceSpecification{
{
DeviceIndex: aws.Int64(providerConfig.DeviceIndex),
AssociatePublicIpAddress: providerConfig.PublicIP,
SubnetId: providerConfig.Subnet.ID,
Groups: []*string{
aws.String("sg-00868b02fbe29de17"),
aws.String("sg-0a4658991dc5eb40a"),
aws.String("sg-009a70e28fa4ba84e"),
aws.String("sg-07323d56fb932c84c"),
aws.String("sg-08b1ffd32874d59a2"),
},
},
},
Placement: &ec2.Placement{
GroupName: aws.String(stubPlacementGroupName),
},
UserData: aws.String(""),
},
},
{
name: "With a Placement PartionNumber with a non Partition placement group",
instancesOutput: stubReservation(stubAMIID, stubInstanceID, "192.168.0.10"),
providerConfig: stubPlacementGroupNumberConfig(1),
succeeds: false,
infra: infra,
objects: []runtime.Object{
stubPlacementGroup(machinev1.AWSClusterPlacementGroupType),
},
},
{
name: "With a Placement PartionNumber with a Partition placement group",
instancesOutput: stubReservation(stubAMIID, stubInstanceID, "192.168.0.10"),
providerConfig: stubPlacementGroupNumberConfig(1),
succeeds: true,
infra: infra,
objects: []runtime.Object{
stubPlacementGroup(machinev1.AWSPartitionPlacementGroupType),
},
runInstancesInput: &ec2.RunInstancesInput{
IamInstanceProfile: &ec2.IamInstanceProfileSpecification{
Name: aws.String(*providerConfig.IAMInstanceProfile.ID),
},
ImageId: aws.String(*providerConfig.AMI.ID),
InstanceType: &providerConfig.InstanceType,
MinCount: aws.Int64(1),
MaxCount: aws.Int64(1),
KeyName: providerConfig.KeyName,
TagSpecifications: []*ec2.TagSpecification{{
ResourceType: aws.String("instance"),
Tags: stubTagListWithInfraObject,
}, {
ResourceType: aws.String("volume"),
Tags: stubTagListWithInfraObject,
}},
NetworkInterfaces: []*ec2.InstanceNetworkInterfaceSpecification{
{
DeviceIndex: aws.Int64(providerConfig.DeviceIndex),
AssociatePublicIpAddress: providerConfig.PublicIP,
SubnetId: providerConfig.Subnet.ID,
Groups: []*string{
aws.String("sg-00868b02fbe29de17"),
aws.String("sg-0a4658991dc5eb40a"),
aws.String("sg-009a70e28fa4ba84e"),
aws.String("sg-07323d56fb932c84c"),
aws.String("sg-08b1ffd32874d59a2"),
},
},
},
Placement: &ec2.Placement{
GroupName: aws.String(stubPlacementGroupName),
PartitionNumber: aws.Int64(1),
},
UserData: aws.String(""),
},
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
Expand Down
42 changes: 5 additions & 37 deletions pkg/actuators/machine/stubs.go
Expand Up @@ -9,7 +9,6 @@ import (
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/aws/aws-sdk-go/service/elbv2"
configv1 "github.com/openshift/api/config/v1"
machinev1 "github.com/openshift/api/machine/v1"
machinev1beta1 "github.com/openshift/api/machine/v1beta1"
machinecontroller "github.com/openshift/machine-api-operator/pkg/controller/machine"
awsclient "github.com/openshift/machine-api-provider-aws/pkg/client"
Expand All @@ -24,12 +23,11 @@ const (
awsCredentialsSecretName = "aws-credentials-secret"
userDataSecretName = "aws-actuator-user-data-secret"

keyName = "aws-actuator-key-name"
stubClusterID = "aws-actuator-cluster"
stubMachineName = "aws-actuator-testing-machine"
stubAMIID = "ami-a9acbbd6"
stubInstanceID = "i-02fcb933c5da7085c"
stubPlacementGroupName = "placement-group-1"
keyName = "aws-actuator-key-name"
stubClusterID = "aws-actuator-cluster"
stubMachineName = "aws-actuator-testing-machine"
stubAMIID = "ami-a9acbbd6"
stubInstanceID = "i-02fcb933c5da7085c"
)

const userDataBlob = `#cloud-config
Expand Down Expand Up @@ -247,36 +245,6 @@ func stubInvalidInstanceTenancy() *machinev1beta1.AWSMachineProviderConfig {
return pc
}

func stubPlacementGroupNameConfig() *machinev1beta1.AWSMachineProviderConfig {
pc := stubProviderConfig()
pc.Placement.Group.Name = stubPlacementGroupName
return pc
}

func stubPlacementGroupNumberConfig(partitionNumber int32) *machinev1beta1.AWSMachineProviderConfig {
pc := stubProviderConfig()
pc.Placement.Group.Name = stubPlacementGroupName
pc.Placement.PartitionNumber = partitionNumber
return pc
}

func stubPlacementGroup(groupType machinev1.AWSPlacementGroupType) *machinev1.AWSPlacementGroup {
return &machinev1.AWSPlacementGroup{
ObjectMeta: metav1.ObjectMeta{
Name: stubPlacementGroupName,
Namespace: defaultNamespace,
},
Spec: machinev1.AWSPlacementGroupSpec{
ManagementSpec: machinev1.AWSPlacementGroupManagementSpec{
ManagementState: machinev1.ManagedManagementState,
Managed: &machinev1.ManagedAWSPlacementGroup{
GroupType: groupType,
},
},
},
}
}

func stubDescribeLoadBalancersOutput() *elbv2.DescribeLoadBalancersOutput {
return &elbv2.DescribeLoadBalancersOutput{
LoadBalancers: []*elbv2.LoadBalancer{
Expand Down
13 changes: 13 additions & 0 deletions pkg/api/machine/v1/common.go
@@ -0,0 +1,13 @@
package v1

// InstanceTenancy indicates if instance should run on shared or single-tenant hardware.
type InstanceTenancy string

const (
// DefaultTenancy instance runs on shared hardware
DefaultTenancy InstanceTenancy = "default"
// DedicatedTenancy instance runs on single-tenant hardware
DedicatedTenancy InstanceTenancy = "dedicated"
// HostTenancy instance runs on a Dedicated Host, which is an isolated server with configurations that you can control.
HostTenancy InstanceTenancy = "host"
)
7 changes: 7 additions & 0 deletions pkg/api/machine/v1/doc.go
@@ -0,0 +1,7 @@
// +k8s:deepcopy-gen=package,register
// +k8s:defaulter-gen=TypeMeta
// +k8s:openapi-gen=true

// +kubebuilder:validation:Optional
// +groupName=machine.openshift.io
package v1
40 changes: 40 additions & 0 deletions pkg/api/machine/v1/register.go
@@ -0,0 +1,40 @@
package v1

import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
)

var (
GroupName = "machine.openshift.io"
GroupVersion = schema.GroupVersion{Group: GroupName, Version: "v1"}
schemeBuilder = runtime.NewSchemeBuilder(addKnownTypes)
// Install is a function which adds this version to a scheme
Install = schemeBuilder.AddToScheme

// SchemeGroupVersion generated code relies on this name
// Deprecated
SchemeGroupVersion = GroupVersion
// AddToScheme exists solely to keep the old generators creating valid code
// DEPRECATED
AddToScheme = schemeBuilder.AddToScheme
)

// Resource generated code relies on this being here, but it logically belongs to the group
// DEPRECATED
func Resource(resource string) schema.GroupResource {
return schema.GroupResource{Group: GroupName, Resource: resource}
}

// Adds the list of known types to api.Scheme.
func addKnownTypes(scheme *runtime.Scheme) error {
metav1.AddToGroupVersion(scheme, GroupVersion)

scheme.AddKnownTypes(GroupVersion,
&AWSPlacementGroup{},
&AWSPlacementGroupList{},
)

return nil
}