diff --git a/cmd/cluster-operator-apiserver/app/testing/server_test.go b/cmd/cluster-operator-apiserver/app/testing/server_test.go index 6573bab2e..3e70d4729 100644 --- a/cmd/cluster-operator-apiserver/app/testing/server_test.go +++ b/cmd/cluster-operator-apiserver/app/testing/server_test.go @@ -54,6 +54,9 @@ func TestRun(t *testing.T) { }, }, }, + Hardware: clusteroperator.ClusterHardwareSpec{ + AWS: &clusteroperator.AWSClusterSpec{}, + }, }, }) if err != nil { diff --git a/contrib/ansible/create-cluster-playbook.yml b/contrib/ansible/create-cluster-playbook.yml index 7b7dc2b5a..25a36aed7 100644 --- a/contrib/ansible/create-cluster-playbook.yml +++ b/contrib/ansible/create-cluster-playbook.yml @@ -31,9 +31,6 @@ # SSH private key used for access to all VMs CO creates: ssh_priv_key: '~/.ssh/libra.pem' - # AWS keypair name to use for cluster machines - aws_ssh_keypair: "libra" - # Force regeneration of cluster cert. Will happen regardless if one does not exist. redeploy_cluster_cert: False @@ -136,7 +133,6 @@ AWS_SECRET_ACCESS_KEY: "{{ l_aws_secret_access_key }}" SSH_PRIVATE_KEY: "{{ l_aws_ssh_private_key }}" SSH_PUBLIC_KEY: "{{ l_aws_ssh_public_key }}" - SSH_KEYPAIR_NAME: "{{ aws_ssh_keypair }}" register: cluster_reg - name: create/update cluster diff --git a/contrib/examples/cluster-deployment-template.yaml b/contrib/examples/cluster-deployment-template.yaml index a29d8bc35..6c5ce35ff 100644 --- a/contrib/examples/cluster-deployment-template.yaml +++ b/contrib/examples/cluster-deployment-template.yaml @@ -40,8 +40,7 @@ parameters: required: true description: Base64 encoded SSH public key that can be used to access the provisioned servers. - name: SSH_KEYPAIR_NAME - required: true - value: libra + required: false description: Name of AWS keypair to use for machines in this cluster. objects: diff --git a/pkg/ansible/generate.go b/pkg/ansible/generate.go index 62638d632..a97b0d936 100644 --- a/pkg/ansible/generate.go +++ b/pkg/ansible/generate.go @@ -222,6 +222,9 @@ all: openshift_aws_users: - key_name: [[ .SSHKeyName ]] pub_key: "{{ lookup('file', '/ansible/ssh/publickey.pub') }}" + + # This will remove the .SSHKeyName keypair from AWS + openshift_aws_enable_uninstall_shared_objects: [[ .UninstallSSHKeyPair ]] # -- # # S3 # @@ -323,6 +326,7 @@ type clusterParams struct { Region string SSHKeyName string SSHUser string + UninstallSSHKeyPair bool VPCDefaults string ELBMasterExternalName string ELBMasterInternalName string @@ -368,6 +372,7 @@ func GenerateClusterWideVars( Region: hardwareSpec.Region, SSHKeyName: hardwareSpec.KeyPairName, SSHUser: hardwareSpec.SSHUser, + UninstallSSHKeyPair: hardwareSpec.KeyPairName == clusterID, // only uninstall the cloud ssh keypair when it's unique to the cluster ELBMasterExternalName: controller.ELBMasterExternalName(clusterID), ELBMasterInternalName: controller.ELBMasterInternalName(clusterID), ELBInfraName: controller.ELBInfraName(clusterID), diff --git a/pkg/ansible/generate_test.go b/pkg/ansible/generate_test.go index ef357bc92..ea7c9eea7 100644 --- a/pkg/ansible/generate_test.go +++ b/pkg/ansible/generate_test.go @@ -29,12 +29,12 @@ const ( imageFormat = "openshift/origin-${component}:v3.10.0" ) -func testClusterSpec() *coapi.ClusterDeploymentSpec { +func testClusterSpec(sshKeyPairName string) *coapi.ClusterDeploymentSpec { return &coapi.ClusterDeploymentSpec{ Hardware: coapi.ClusterHardwareSpec{ AWS: &coapi.AWSClusterSpec{ Region: "us-east-1", - KeyPairName: "mykey", + KeyPairName: sshKeyPairName, SSHUser: "centos", }, }, @@ -67,7 +67,7 @@ func TestGenerateClusterWideVars(t *testing.T) { { name: "cluster", clusterID: "testcluster", - clusterSpec: testClusterSpec(), + clusterSpec: testClusterSpec("testcluster"), infraSize: 2, clusterVersion: testClusterVersion(), sdnPluginName: "fakeplugin", @@ -79,7 +79,7 @@ func TestGenerateClusterWideVars(t *testing.T) { "openshift_aws_elb_master_external_name: testcluster-cp-ext", "openshift_aws_elb_master_internal_name: testcluster-cp-int", "openshift_aws_elb_infra_name: testcluster-infra", - "openshift_aws_ssh_key_name: mykey", + "openshift_aws_ssh_key_name: testcluster", "openshift_aws_region: us-east-1", "ansible_ssh_user: centos", "openshift_deployment_type: origin", @@ -87,6 +87,7 @@ func TestGenerateClusterWideVars(t *testing.T) { "openshift_portal_net: 172.30.0.0/16", "osm_cluster_network_cidr: 10.128.0.0/14", "os_sdn_network_plugin_name: \"fakeplugin\"", + "openshift_aws_enable_uninstall_shared_objects: true", }, shouldNotInclude: []string{ "openshift_release", @@ -100,7 +101,7 @@ func TestGenerateClusterWideVars(t *testing.T) { { name: "long clusterID", clusterID: "012345678901234567890123456789-abcde", - clusterSpec: testClusterSpec(), + clusterSpec: testClusterSpec("not_same_as_clusterid"), infraSize: 2, clusterVersion: testClusterVersion(), serviceCIDRs: capiv1.NetworkRanges{CIDRBlocks: []string{"172.30.0.0/16"}}, @@ -110,6 +111,8 @@ func TestGenerateClusterWideVars(t *testing.T) { "openshift_aws_elb_master_external_name: 0123456789012345678-abcde-cp-ext", "openshift_aws_elb_master_internal_name: 0123456789012345678-abcde-cp-int", "openshift_aws_elb_infra_name: 0123456789012345678-abcde-infra", + "openshift_aws_ssh_key_name: not_same_as_clusterid", + "openshift_aws_enable_uninstall_shared_objects: false", }, }, } diff --git a/pkg/registry/clusteroperator/clusterdeployment/storage_test.go b/pkg/registry/clusteroperator/clusterdeployment/storage_test.go index 001d870e2..452aefdfd 100644 --- a/pkg/registry/clusteroperator/clusterdeployment/storage_test.go +++ b/pkg/registry/clusteroperator/clusterdeployment/storage_test.go @@ -71,6 +71,9 @@ func validNewClusterDeployment(name string) *clusteroperatorapi.ClusterDeploymen Pods: capiv1.NetworkRanges{CIDRBlocks: []string{"10.128.0.0/14"}}, ServiceDomain: "svc.cluster.local", }, + Hardware: clusteroperatorapi.ClusterHardwareSpec{ + AWS: &clusteroperatorapi.AWSClusterSpec{}, + }, }, } } @@ -92,6 +95,11 @@ func TestCreate(t *testing.T) { // invalid &clusteroperatorapi.ClusterDeployment{ ObjectMeta: metav1.ObjectMeta{Name: "*BadName!"}, + Spec: clusteroperatorapi.ClusterDeploymentSpec{ + Hardware: clusteroperatorapi.ClusterHardwareSpec{ + AWS: &clusteroperatorapi.AWSClusterSpec{}, + }, + }, }, ) } diff --git a/pkg/registry/clusteroperator/clusterdeployment/strategy.go b/pkg/registry/clusteroperator/clusterdeployment/strategy.go index 21d317db7..63a687eac 100644 --- a/pkg/registry/clusteroperator/clusterdeployment/strategy.go +++ b/pkg/registry/clusteroperator/clusterdeployment/strategy.go @@ -91,6 +91,11 @@ func (clusterDeploymentRESTStrategy) PrepareForCreate(ctx genericapirequest.Cont clusterDeployment.Spec.ClusterName = clusterDeployment.Name + "-" + utilrand.String(5) + if clusterDeployment.Spec.Hardware.AWS.KeyPairName == "" { + // This is how the KeyPairName is generated. + clusterDeployment.Spec.Hardware.AWS.KeyPairName = clusterDeployment.Spec.ClusterName + } + // Creating a brand new object, thus it must have no // status. We can't fail here if they passed a status in, so // we just wipe it clean. diff --git a/pkg/registry/clusteroperator/clusterdeployment/strategy_test.go b/pkg/registry/clusteroperator/clusterdeployment/strategy_test.go index 2d1666599..9a4d289b6 100644 --- a/pkg/registry/clusteroperator/clusterdeployment/strategy_test.go +++ b/pkg/registry/clusteroperator/clusterdeployment/strategy_test.go @@ -19,6 +19,8 @@ package clusterdeployment import ( "testing" + "github.com/stretchr/testify/assert" + clusteroperator "github.com/openshift/cluster-operator/pkg/apis/clusteroperator" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -50,13 +52,58 @@ func TestClusterDeploymentStrategyTrivial(t *testing.T) { } } -// TestClusterDeploymentCreate -func TestClusterDeploymentCreate(t *testing.T) { - // Create a clusterdeployment or clusterdeployments - clusterDeployment := &clusteroperator.ClusterDeployment{} +// TestClusterDeploymentCreateWithSshKeyName +func TestClusterDeploymentCreateWithSshKeyName(t *testing.T) { + cases := []struct { + name string + clusterDeploymentName string + keyPairName string + keyPairNameGenerationExpected bool + }{ + { + name: "specify ssh keypair name", + clusterDeploymentName: "testcluster", + keyPairName: "TEST KEYPAIR NAME", + keyPairNameGenerationExpected: false, + }, + { + name: "generate ssh keypair name", + clusterDeploymentName: "testcluster", + keyPairNameGenerationExpected: true, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + // Arrange + // Create a clusterdeployment or clusterdeployments + clusterDeployment := &clusteroperator.ClusterDeployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: tc.clusterDeploymentName, + }, + Spec: clusteroperator.ClusterDeploymentSpec{ + Hardware: clusteroperator.ClusterHardwareSpec{ + AWS: &clusteroperator.AWSClusterSpec{ + KeyPairName: tc.keyPairName, + }, + }, + }, + } + + // Act + // Canonicalize the cluster + clusterDeploymentRESTStrategies.PrepareForCreate(nil, clusterDeployment) - // Canonicalize the cluster - clusterDeploymentRESTStrategies.PrepareForCreate(nil, clusterDeployment) + // Assert + assert.Regexp(t, "^testcluster-\\w\\w\\w\\w\\w$", clusterDeployment.Spec.ClusterName) + + if tc.keyPairNameGenerationExpected { + assert.Equal(t, clusterDeployment.Spec.ClusterName, clusterDeployment.Spec.Hardware.AWS.KeyPairName) + } else { + assert.Equal(t, tc.keyPairName, clusterDeployment.Spec.Hardware.AWS.KeyPairName) + } + }) + } } // TestClusterDeploymentUpdate tests that generation is incremented correctly when the