From fbb9bd7ddadf7bac146d2d2983acc35144614bb7 Mon Sep 17 00:00:00 2001 From: Ole Markus With Date: Tue, 28 Apr 2020 14:45:05 +0200 Subject: [PATCH] Fix various tests that used masters without etcd members --- pkg/apis/kops/validation/instancegroup.go | 4 +- .../kops/validation/instancegroup_test.go | 4 +- ...masters.existing-iam.example.com_user_data | 2 +- ...masters.existing-iam.example.com_user_data | 2 +- ...masters.existing-iam.example.com_user_data | 2 +- .../existing_iam/in-v1alpha2.yaml | 8 +++ .../update_cluster/existing_iam/kubernetes.tf | 60 ++++++++++++++++++- upup/pkg/fi/cloudup/deepvalidate_test.go | 34 ++++++++++- .../fi/cloudup/populateinstancegroup_test.go | 6 +- upup/pkg/fi/cloudup/validation_test.go | 14 ++--- 10 files changed, 112 insertions(+), 24 deletions(-) diff --git a/pkg/apis/kops/validation/instancegroup.go b/pkg/apis/kops/validation/instancegroup.go index d14cad01d5b85..3b79d0ef80d00 100644 --- a/pkg/apis/kops/validation/instancegroup.go +++ b/pkg/apis/kops/validation/instancegroup.go @@ -216,7 +216,7 @@ func CrossValidateInstanceGroup(g *kops.InstanceGroup, cluster *kops.Cluster) fi func ValidateMasterInstanceGroup(g *kops.InstanceGroup, cluster *kops.Cluster) field.ErrorList { allErrs := field.ErrorList{} - for _, etcd := range cluster.Spec.EtcdClusters { + for k, etcd := range cluster.Spec.EtcdClusters { hasEtcd := false for _, m := range etcd.Members { if fi.StringValue(m.InstanceGroup) == g.ObjectMeta.Name { @@ -225,7 +225,7 @@ func ValidateMasterInstanceGroup(g *kops.InstanceGroup, cluster *kops.Cluster) f } } if !hasEtcd { - allErrs = append(allErrs, field.NotFound(field.NewPath("spec", "etcdClusters").Key(etcd.Name), g.ObjectMeta.Name)) + allErrs = append(allErrs, field.NotFound(field.NewPath("spec", "etcdClusters").Index(k).Child("members").Key("*").Child("instanceGroup"), g.ObjectMeta.Name)) } } return allErrs diff --git a/pkg/apis/kops/validation/instancegroup_test.go b/pkg/apis/kops/validation/instancegroup_test.go index cd4fabf728b7b..947d7d51c540c 100644 --- a/pkg/apis/kops/validation/instancegroup_test.go +++ b/pkg/apis/kops/validation/instancegroup_test.go @@ -103,7 +103,7 @@ func TestValidMasterInstanceGroup(t *testing.T) { Cluster: &kops.Cluster{ Spec: kops.ClusterSpec{ EtcdClusters: []*kops.EtcdClusterSpec{ - &kops.EtcdClusterSpec{ + { Name: "main", Members: []*kops.EtcdMemberSpec{ { @@ -138,7 +138,7 @@ func TestValidMasterInstanceGroup(t *testing.T) { Cluster: &kops.Cluster{ Spec: kops.ClusterSpec{ EtcdClusters: []*kops.EtcdClusterSpec{ - &kops.EtcdClusterSpec{ + { Name: "main", Members: []*kops.EtcdMemberSpec{ { diff --git a/tests/integration/update_cluster/existing_iam/data/aws_launch_configuration_master-us-test-1a.masters.existing-iam.example.com_user_data b/tests/integration/update_cluster/existing_iam/data/aws_launch_configuration_master-us-test-1a.masters.existing-iam.example.com_user_data index 97eed3c18eef1..535827c7b8315 100644 --- a/tests/integration/update_cluster/existing_iam/data/aws_launch_configuration_master-us-test-1a.masters.existing-iam.example.com_user_data +++ b/tests/integration/update_cluster/existing_iam/data/aws_launch_configuration_master-us-test-1a.masters.existing-iam.example.com_user_data @@ -153,7 +153,7 @@ etcdClusters: kubeAPIServer: allowPrivileged: true anonymousAuth: false - apiServerCount: 1 + apiServerCount: 3 authorizationMode: AlwaysAllow bindAddress: 0.0.0.0 cloudProvider: aws diff --git a/tests/integration/update_cluster/existing_iam/data/aws_launch_configuration_master-us-test-1b.masters.existing-iam.example.com_user_data b/tests/integration/update_cluster/existing_iam/data/aws_launch_configuration_master-us-test-1b.masters.existing-iam.example.com_user_data index 7ece69c523a7b..628fe1a66d25e 100644 --- a/tests/integration/update_cluster/existing_iam/data/aws_launch_configuration_master-us-test-1b.masters.existing-iam.example.com_user_data +++ b/tests/integration/update_cluster/existing_iam/data/aws_launch_configuration_master-us-test-1b.masters.existing-iam.example.com_user_data @@ -153,7 +153,7 @@ etcdClusters: kubeAPIServer: allowPrivileged: true anonymousAuth: false - apiServerCount: 1 + apiServerCount: 3 authorizationMode: AlwaysAllow bindAddress: 0.0.0.0 cloudProvider: aws diff --git a/tests/integration/update_cluster/existing_iam/data/aws_launch_configuration_master-us-test-1c.masters.existing-iam.example.com_user_data b/tests/integration/update_cluster/existing_iam/data/aws_launch_configuration_master-us-test-1c.masters.existing-iam.example.com_user_data index 41c01b84fa236..1e0d47f35d4c3 100644 --- a/tests/integration/update_cluster/existing_iam/data/aws_launch_configuration_master-us-test-1c.masters.existing-iam.example.com_user_data +++ b/tests/integration/update_cluster/existing_iam/data/aws_launch_configuration_master-us-test-1c.masters.existing-iam.example.com_user_data @@ -153,7 +153,7 @@ etcdClusters: kubeAPIServer: allowPrivileged: true anonymousAuth: false - apiServerCount: 1 + apiServerCount: 3 authorizationMode: AlwaysAllow bindAddress: 0.0.0.0 cloudProvider: aws diff --git a/tests/integration/update_cluster/existing_iam/in-v1alpha2.yaml b/tests/integration/update_cluster/existing_iam/in-v1alpha2.yaml index dd4860ab2de54..a57f3e6e4a743 100644 --- a/tests/integration/update_cluster/existing_iam/in-v1alpha2.yaml +++ b/tests/integration/update_cluster/existing_iam/in-v1alpha2.yaml @@ -13,10 +13,18 @@ spec: - etcdMembers: - instanceGroup: master-us-test-1a name: a + - instanceGroup: master-us-test-1b + name: b + - instanceGroup: master-us-test-1c + name: c name: main - etcdMembers: - instanceGroup: master-us-test-1a name: a + - instanceGroup: master-us-test-1b + name: b + - instanceGroup: master-us-test-1c + name: c name: events kubelet: anonymousAuth: false diff --git a/tests/integration/update_cluster/existing_iam/kubernetes.tf b/tests/integration/update_cluster/existing_iam/kubernetes.tf index 62ec92de281e3..6d0a7eb71ef93 100644 --- a/tests/integration/update_cluster/existing_iam/kubernetes.tf +++ b/tests/integration/update_cluster/existing_iam/kubernetes.tf @@ -217,7 +217,7 @@ resource "aws_ebs_volume" "a-etcd-events-existing-iam-example-com" { tags = { "KubernetesCluster" = "existing-iam.example.com" "Name" = "a.etcd-events.existing-iam.example.com" - "k8s.io/etcd/events" = "a/a" + "k8s.io/etcd/events" = "a/a,b,c" "k8s.io/role/master" = "1" "kubernetes.io/cluster/existing-iam.example.com" = "owned" } @@ -231,7 +231,63 @@ resource "aws_ebs_volume" "a-etcd-main-existing-iam-example-com" { tags = { "KubernetesCluster" = "existing-iam.example.com" "Name" = "a.etcd-main.existing-iam.example.com" - "k8s.io/etcd/main" = "a/a" + "k8s.io/etcd/main" = "a/a,b,c" + "k8s.io/role/master" = "1" + "kubernetes.io/cluster/existing-iam.example.com" = "owned" + } + type = "gp2" +} + +resource "aws_ebs_volume" "b-etcd-events-existing-iam-example-com" { + availability_zone = "us-test-1b" + encrypted = false + size = 20 + tags = { + "KubernetesCluster" = "existing-iam.example.com" + "Name" = "b.etcd-events.existing-iam.example.com" + "k8s.io/etcd/events" = "b/a,b,c" + "k8s.io/role/master" = "1" + "kubernetes.io/cluster/existing-iam.example.com" = "owned" + } + type = "gp2" +} + +resource "aws_ebs_volume" "b-etcd-main-existing-iam-example-com" { + availability_zone = "us-test-1b" + encrypted = false + size = 20 + tags = { + "KubernetesCluster" = "existing-iam.example.com" + "Name" = "b.etcd-main.existing-iam.example.com" + "k8s.io/etcd/main" = "b/a,b,c" + "k8s.io/role/master" = "1" + "kubernetes.io/cluster/existing-iam.example.com" = "owned" + } + type = "gp2" +} + +resource "aws_ebs_volume" "c-etcd-events-existing-iam-example-com" { + availability_zone = "us-test-1c" + encrypted = false + size = 20 + tags = { + "KubernetesCluster" = "existing-iam.example.com" + "Name" = "c.etcd-events.existing-iam.example.com" + "k8s.io/etcd/events" = "c/a,b,c" + "k8s.io/role/master" = "1" + "kubernetes.io/cluster/existing-iam.example.com" = "owned" + } + type = "gp2" +} + +resource "aws_ebs_volume" "c-etcd-main-existing-iam-example-com" { + availability_zone = "us-test-1c" + encrypted = false + size = 20 + tags = { + "KubernetesCluster" = "existing-iam.example.com" + "Name" = "c.etcd-main.existing-iam.example.com" + "k8s.io/etcd/main" = "c/a,b,c" "k8s.io/role/master" = "1" "kubernetes.io/cluster/existing-iam.example.com" = "owned" } diff --git a/upup/pkg/fi/cloudup/deepvalidate_test.go b/upup/pkg/fi/cloudup/deepvalidate_test.go index f479a7e97223b..d77785729ba62 100644 --- a/upup/pkg/fi/cloudup/deepvalidate_test.go +++ b/upup/pkg/fi/cloudup/deepvalidate_test.go @@ -118,10 +118,12 @@ func TestDeepValidate_ExtraMasterZone(t *testing.T) { {Name: "mock1b", Zone: "us-mock-1b", CIDR: "172.20.2.0/24"}, } var groups []*kopsapi.InstanceGroup - groups = append(groups, buildMinimalMasterInstanceGroup("subnet-us-mock-1a", "subnet-us-mock-1b", "subnet-us-mock-1c")) + groups = append(groups, buildMinimalMasterInstanceGroup("subnet-us-mock-1a")) + groups = append(groups, buildMinimalMasterInstanceGroup("subnet-us-mock-1b")) + groups = append(groups, buildMinimalMasterInstanceGroup("subnet-us-mock-1c")) groups = append(groups, buildMinimalNodeInstanceGroup("subnet-us-mock-1a", "subnet-us-mock-1b")) - expectErrorFromDeepValidate(t, c, groups, "[spec.subnets[0]: Not found: \"subnet-us-mock-1a\", spec.subnets[1]: Not found: \"subnet-us-mock-1b\", spec.subnets[2]: Not found: \"subnet-us-mock-1c\"]") + expectErrorFromDeepValidate(t, c, groups, "spec.subnets[0]: Not found: \"subnet-us-mock-1a\"") } func TestDeepValidate_EvenEtcdClusterSize(t *testing.T) { @@ -137,12 +139,38 @@ func TestDeepValidate_EvenEtcdClusterSize(t *testing.T) { } var groups []*kopsapi.InstanceGroup - groups = append(groups, buildMinimalMasterInstanceGroup("subnet-us-mock-1a", "subnet-us-mock-1b", "subnet-us-mock-1c", "subnet-us-mock-1d")) + groups = append(groups, buildMinimalMasterInstanceGroup("subnet-us-mock-1a")) + groups = append(groups, buildMinimalMasterInstanceGroup("subnet-us-mock-1b")) + groups = append(groups, buildMinimalMasterInstanceGroup("subnet-us-mock-1c")) + groups = append(groups, buildMinimalMasterInstanceGroup("subnet-us-mock-1d")) groups = append(groups, buildMinimalNodeInstanceGroup("subnet-us-mock-1a")) expectErrorFromDeepValidate(t, c, groups, "Should be an odd number of master-zones for quorum. Use --zones and --master-zones to declare node zones and master zones separately") } +func TestDeepValidate_MissingEtcdMember(t *testing.T) { + c := buildDefaultCluster(t) + c.Spec.EtcdClusters = []*kopsapi.EtcdClusterSpec{ + { + Name: "main", + Members: []*kopsapi.EtcdMemberSpec{ + {Name: "us-mock-1a", InstanceGroup: fi.String("us-mock-1a")}, + {Name: "us-mock-1b", InstanceGroup: fi.String("us-mock-1b")}, + {Name: "us-mock-1c", InstanceGroup: fi.String("us-mock-1c")}, + }, + }, + } + + var groups []*kopsapi.InstanceGroup + groups = append(groups, buildMinimalMasterInstanceGroup("subnet-us-mock-1a")) + groups = append(groups, buildMinimalMasterInstanceGroup("subnet-us-mock-1b")) + groups = append(groups, buildMinimalMasterInstanceGroup("subnet-us-mock-1c")) + groups = append(groups, buildMinimalMasterInstanceGroup("subnet-us-mock-1d")) + groups = append(groups, buildMinimalNodeInstanceGroup("subnet-us-mock-1a")) + + expectErrorFromDeepValidate(t, c, groups, "spec.etcdClusters[0].members[*].instanceGroup: Not found: \"master-subnet-us-mock-1a\"") +} + func expectErrorFromDeepValidate(t *testing.T, c *kopsapi.Cluster, groups []*kopsapi.InstanceGroup, message string) { err := validation.DeepValidate(c, groups, true) if err == nil { diff --git a/upup/pkg/fi/cloudup/populateinstancegroup_test.go b/upup/pkg/fi/cloudup/populateinstancegroup_test.go index a28e11e040739..079df7a40bfa3 100644 --- a/upup/pkg/fi/cloudup/populateinstancegroup_test.go +++ b/upup/pkg/fi/cloudup/populateinstancegroup_test.go @@ -33,11 +33,11 @@ func buildMinimalNodeInstanceGroup(subnets ...string) *kopsapi.InstanceGroup { return g } -func buildMinimalMasterInstanceGroup(subnets ...string) *kopsapi.InstanceGroup { +func buildMinimalMasterInstanceGroup(subnet string) *kopsapi.InstanceGroup { g := &kopsapi.InstanceGroup{} - g.ObjectMeta.Name = "master" + g.ObjectMeta.Name = "master-" + subnet g.Spec.Role = kopsapi.InstanceGroupRoleMaster - g.Spec.Subnets = subnets + g.Spec.Subnets = []string{subnet} return g } diff --git a/upup/pkg/fi/cloudup/validation_test.go b/upup/pkg/fi/cloudup/validation_test.go index 0e17d5cb0eb8c..ae82872bcc764 100644 --- a/upup/pkg/fi/cloudup/validation_test.go +++ b/upup/pkg/fi/cloudup/validation_test.go @@ -21,7 +21,6 @@ import ( "strings" "testing" - "k8s.io/apimachinery/pkg/util/sets" "k8s.io/klog" api "k8s.io/kops/pkg/apis/kops" "k8s.io/kops/pkg/apis/kops/validation" @@ -39,23 +38,20 @@ func buildDefaultCluster(t *testing.T) *api.Cluster { } if len(c.Spec.EtcdClusters) == 0 { - zones := sets.NewString() - for _, z := range c.Spec.Subnets { - zones.Insert(z.Zone) - } - etcdZones := zones.List() for _, etcdCluster := range EtcdClusters { + etcd := &api.EtcdClusterSpec{} etcd.Name = etcdCluster - for _, zone := range etcdZones { + for _, subnet := range c.Spec.Subnets { m := &api.EtcdMemberSpec{} - m.Name = zone - m.InstanceGroup = fi.String(zone) + m.Name = subnet.Zone + m.InstanceGroup = fi.String("master-" + subnet.Name) etcd.Members = append(etcd.Members, m) } c.Spec.EtcdClusters = append(c.Spec.EtcdClusters, etcd) } + } fullSpec, err := mockedPopulateClusterSpec(c)