From 7352f30783faa7cda1f43e2fd09c2e4b2d793e28 Mon Sep 17 00:00:00 2001 From: justinsb Date: Wed, 19 Jul 2023 10:54:02 -0400 Subject: [PATCH 1/3] kubetest2-kops: rename control-plane-size flag to control-plane-count For consistency with kops create cluster. It doesn't appear to be used outside this repo, although test-infra declares KOPS_CONTROL_PLANE_SIZE and that is then passed to our e2e/scenarios scripts. Issue #15559 --- tests/e2e/kubetest2-kops/deployer/common.go | 4 +-- tests/e2e/kubetest2-kops/deployer/deployer.go | 29 ++++++++++++++----- tests/e2e/kubetest2-kops/deployer/up.go | 4 +-- tests/e2e/scenarios/upgrade-ab/run-test.sh | 2 +- 4 files changed, 26 insertions(+), 13 deletions(-) diff --git a/tests/e2e/kubetest2-kops/deployer/common.go b/tests/e2e/kubetest2-kops/deployer/common.go index 2952427fb7..9dc7341f5d 100644 --- a/tests/e2e/kubetest2-kops/deployer/common.go +++ b/tests/e2e/kubetest2-kops/deployer/common.go @@ -157,8 +157,8 @@ func (d *deployer) verifyKopsFlags() error { return errors.New("missing required --kops-binary-path when --kops-version-marker is not used") } - if d.ControlPlaneSize == 0 { - d.ControlPlaneSize = 1 + if d.ControlPlaneCount == 0 { + d.ControlPlaneCount = 1 } switch d.CloudProvider { diff --git a/tests/e2e/kubetest2-kops/deployer/deployer.go b/tests/e2e/kubetest2-kops/deployer/deployer.go index 5ad0289636..76db5c13bf 100644 --- a/tests/e2e/kubetest2-kops/deployer/deployer.go +++ b/tests/e2e/kubetest2-kops/deployer/deployer.go @@ -48,14 +48,16 @@ type deployer struct { KopsBaseURL string `flag:"-"` PublishVersionMarker string `flag:"publish-version-marker" desc:"The GCS path to which the --kops-version-marker is uploaded if the tests pass"` - ClusterName string `flag:"cluster-name" desc:"The FQDN to use for the cluster name"` - ControlPlaneSize int `flag:"control-plane-size" desc:"Number of control plane instances"` - CloudProvider string `flag:"cloud-provider" desc:"Which cloud provider to use"` - GCPProject string `flag:"gcp-project" desc:"Which GCP Project to use when --cloud-provider=gce"` - Env []string `flag:"env" desc:"Additional env vars to set for kops commands in NAME=VALUE format"` - CreateArgs string `flag:"create-args" desc:"Extra space-separated arguments passed to 'kops create cluster'"` - KopsBinaryPath string `flag:"kops-binary-path" desc:"The path to kops executable used for testing"` - createBucket bool `flag:"-"` + ClusterName string `flag:"cluster-name" desc:"The FQDN to use for the cluster name"` + CloudProvider string `flag:"cloud-provider" desc:"Which cloud provider to use"` + GCPProject string `flag:"gcp-project" desc:"Which GCP Project to use when --cloud-provider=gce"` + Env []string `flag:"env" desc:"Additional env vars to set for kops commands in NAME=VALUE format"` + CreateArgs string `flag:"create-args" desc:"Extra space-separated arguments passed to 'kops create cluster'"` + KopsBinaryPath string `flag:"kops-binary-path" desc:"The path to kops executable used for testing"` + createBucket bool `flag:"-"` + + // ControlPlaneCount specifies the number of VMs in the control-plane. + ControlPlaneCount int `flag:"control-plane-count" desc:"Number of control plane instances"` ControlPlaneIGOverrides []string `flag:"control-plane-instance-group-overrides" desc:"overrides for the control plane instance groups"` NodeIGOverrides []string `flag:"node-instance-group-overrides" desc:"overrides for the node instance groups"` @@ -123,6 +125,17 @@ func New(opts types.Options) (types.Deployer, *pflag.FlagSet) { // register flags for klog klog.InitFlags(nil) fs.AddGoFlagSet(flag.CommandLine) + + // Map deprecated flag names to their new names + fs.SetNormalizeFunc(func(f *pflag.FlagSet, name string) pflag.NormalizedName { + switch name { + case "control-plane-size": + klog.Warningf("deprecated --control-plane-size specified; please use --control-plane-count instead") + name = "control-plane-count" + } + return pflag.NormalizedName(name) + }) + return d, fs } diff --git a/tests/e2e/kubetest2-kops/deployer/up.go b/tests/e2e/kubetest2-kops/deployer/up.go index c8a3eef8b7..abed1de8b5 100644 --- a/tests/e2e/kubetest2-kops/deployer/up.go +++ b/tests/e2e/kubetest2-kops/deployer/up.go @@ -134,7 +134,7 @@ func (d *deployer) createCluster(zones []string, adminAccess string, yes bool) e args = append(args, createArgs...) } args = appendIfUnset(args, "--admin-access", adminAccess) - args = appendIfUnset(args, "--master-count", fmt.Sprintf("%d", d.ControlPlaneSize)) + args = appendIfUnset(args, "--control-plane-count", fmt.Sprintf("%d", d.ControlPlaneCount)) args = appendIfUnset(args, "--master-volume-size", "48") args = appendIfUnset(args, "--node-count", "4") args = appendIfUnset(args, "--node-volume-size", "48") @@ -284,7 +284,7 @@ func (d *deployer) verifyUpFlags() error { func (d *deployer) zones() ([]string, error) { switch d.CloudProvider { case "aws": - return aws.RandomZones(d.ControlPlaneSize) + return aws.RandomZones(d.ControlPlaneCount) case "gce": return gce.RandomZones(1) case "digitalocean": diff --git a/tests/e2e/scenarios/upgrade-ab/run-test.sh b/tests/e2e/scenarios/upgrade-ab/run-test.sh index f6a7e6e438..c4162b50c5 100755 --- a/tests/e2e/scenarios/upgrade-ab/run-test.sh +++ b/tests/e2e/scenarios/upgrade-ab/run-test.sh @@ -87,7 +87,7 @@ ${KUBETEST2} \ --up \ --kops-binary-path="${KOPS_A}" \ --kubernetes-version="${K8S_VERSION_A}" \ - --control-plane-size="${KOPS_CONTROL_PLANE_SIZE:-1}" \ + --control-plane-count="${KOPS_CONTROL_PLANE_SIZE:-1}" \ --template-path="${KOPS_TEMPLATE:-}" \ --create-args="--networking calico ${KOPS_EXTRA_FLAGS:-} ${create_args}" From 3b834a2c87412da3bc3bbd9b8982faad849ebb73 Mon Sep 17 00:00:00 2001 From: justinsb Date: Wed, 19 Jul 2023 11:00:07 -0400 Subject: [PATCH 2/3] test scenarios: recognize KOPS_CONTROL_PLANE_COUNT We still accept KOPS_CONTROL_PLANE_SIZE (for now), but we should be switching to KOPS_CONTROL_PLANE_COUNT --- tests/e2e/scenarios/lib/common.sh | 8 +++++++- tests/e2e/scenarios/upgrade-ab/run-test.sh | 8 +++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/tests/e2e/scenarios/lib/common.sh b/tests/e2e/scenarios/lib/common.sh index a0593bc2d3..6d2614d1e3 100644 --- a/tests/e2e/scenarios/lib/common.sh +++ b/tests/e2e/scenarios/lib/common.sh @@ -137,11 +137,17 @@ function kops-up() { create_args="${create_args} --discovery-store=${DISCOVERY_STORE}/${CLUSTER_NAME}/discovery" fi + # TODO: Switch scripts to use KOPS_CONTROL_PLANE_COUNT + if [[ -n "${KOPS_CONTROL_PLANE_SIZE:-}" ]]; then + echo "Recognized (deprecated) KOPS_CONTROL_PLANE_SIZE=${KOPS_CONTROL_PLANE_SIZE}, please set KOPS_CONTROL_PLANE_COUNT instead" + KOPS_CONTROL_PLANE_COUNT=${KOPS_CONTROL_PLANE_SIZE} + fi + ${KUBETEST2} \ --up \ --kops-binary-path="${KOPS}" \ --kubernetes-version="${K8S_VERSION}" \ --create-args="${create_args}" \ - --control-plane-size="${KOPS_CONTROL_PLANE_SIZE:-1}" \ + --control-plane-count="${KOPS_CONTROL_PLANE_COUNT:-1}" \ --template-path="${KOPS_TEMPLATE-}" } \ No newline at end of file diff --git a/tests/e2e/scenarios/upgrade-ab/run-test.sh b/tests/e2e/scenarios/upgrade-ab/run-test.sh index c4162b50c5..98308da17f 100755 --- a/tests/e2e/scenarios/upgrade-ab/run-test.sh +++ b/tests/e2e/scenarios/upgrade-ab/run-test.sh @@ -83,11 +83,17 @@ if [[ ${KOPS_IRSA-} = true ]]; then create_args="${create_args} --discovery-store=${DISCOVERY_STORE}/${CLUSTER_NAME}/discovery" fi +# TODO: Switch scripts to use KOPS_CONTROL_PLANE_COUNT +if [[ -n "${KOPS_CONTROL_PLANE_SIZE:-}" ]]; then + echo "Recognized (deprecated) KOPS_CONTROL_PLANE_SIZE=${KOPS_CONTROL_PLANE_SIZE}, please set KOPS_CONTROL_PLANE_COUNT instead" + KOPS_CONTROL_PLANE_COUNT=${KOPS_CONTROL_PLANE_SIZE} +fi + ${KUBETEST2} \ --up \ --kops-binary-path="${KOPS_A}" \ --kubernetes-version="${K8S_VERSION_A}" \ - --control-plane-count="${KOPS_CONTROL_PLANE_SIZE:-1}" \ + --control-plane-count="${KOPS_CONTROL_PLANE_COUNT:-1}" \ --template-path="${KOPS_TEMPLATE:-}" \ --create-args="--networking calico ${KOPS_EXTRA_FLAGS:-} ${create_args}" From 53385bc0b466db464b699dd8f0e640f3f9fd3839 Mon Sep 17 00:00:00 2001 From: Ciprian Hacman Date: Fri, 21 Jul 2023 05:24:03 +0300 Subject: [PATCH 3/3] Apply suggestions from code review Co-authored-by: John Gardiner Myers --- tests/e2e/kubetest2-kops/deployer/deployer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/e2e/kubetest2-kops/deployer/deployer.go b/tests/e2e/kubetest2-kops/deployer/deployer.go index 76db5c13bf..7d88b75d80 100644 --- a/tests/e2e/kubetest2-kops/deployer/deployer.go +++ b/tests/e2e/kubetest2-kops/deployer/deployer.go @@ -57,7 +57,7 @@ type deployer struct { createBucket bool `flag:"-"` // ControlPlaneCount specifies the number of VMs in the control-plane. - ControlPlaneCount int `flag:"control-plane-count" desc:"Number of control plane instances"` + ControlPlaneCount int `flag:"control-plane-count" desc:"Number of control-plane instances"` ControlPlaneIGOverrides []string `flag:"control-plane-instance-group-overrides" desc:"overrides for the control plane instance groups"` NodeIGOverrides []string `flag:"node-instance-group-overrides" desc:"overrides for the node instance groups"`