prow-build-canary-cluster: provisioning scripts #5063

pkprzekwas · 2023-03-31T16:17:08Z

As we started migrating Prow jobs to EKS Prow, applying infrastructure changes on that cluster became risky. For that reason, here are changes for conditionally provisioning canary cluster.

This new cluster is meant for:

testing changes before applying on main EKS Prow Cluster,
playground for new solutions.

Can be considered as part of #4686

/assign @xmudrii @ameukam

…uster

pkprzekwas · 2023-04-03T06:53:13Z

/assign @pkprzekwas

pkprzekwas · 2023-04-03T08:31:29Z

/hold

After discussing with @xmudrii , I want to refactor this a bit.

…ster

infra/aws/terraform/prow-build-cluster/Makefile

infra/aws/terraform/prow-build-cluster/eks.tf

infra/aws/terraform/prow-build-cluster/tfbackends/canary.tfbackend

infra/aws/terraform/prow-build-cluster/tfbackends/prod.tfbackend

infra/aws/terraform/prow-build-cluster/terraform.canary.tfvars

infra/aws/terraform/prow-build-cluster/README.md

pkprzekwas · 2023-04-03T14:05:25Z

/unhold

xmudrii

Just two nits, otherwise LGTM.

infra/aws/terraform/prow-build-cluster/kubernetes.tf

infra/aws/terraform/prow-build-cluster/terraform.canary.tfvars

infra/aws/terraform/prow-build-cluster/Makefile

ameukam · 2023-04-03T17:37:10Z

infra/aws/terraform/prow-build-cluster/main.tf

+data "aws_availability_zones" "available" {}
+
+locals {
+  canary_prefix = terraform.workspace != "prod" ? "canary-" : ""


we want to be consistent about prefixes added:

Suggested change

canary_prefix = terraform.workspace != "prod" ? "canary-" : ""

canary_prefix = terraform.workspace != "prod-" ? "canary-" : ""

I also prefer we have a prefix variable defining the environment.

I was thinking about that, but unfortunately we haven't planned that in advance. The production cluster has been already created and introducing prod- prefix would require re-provisioning.

ameukam · 2023-04-03T17:38:08Z

infra/aws/terraform/prow-build-cluster/prow.tf


 # Recognize federated identities from the prow trusted cluster
 resource "aws_iam_openid_connect_provider" "k8s_prow" {
+  count = terraform.workspace == "prod" ? 1 : 0


we are not creating a OIDC provider for the canary env ?

No, we don't have Prow on the canary environment. We use it only for testing infra changes, not Prow changes. That would require some additional effort if we want to go that route.

ameukam · 2023-04-03T17:41:36Z

infra/aws/terraform/prow-build-cluster/terraform.canary.tfvars

+vpc_intra_subnet          = ["10.5.0.0/18", "10.5.64.0/18", "10.5.128.0/18"]
+
+# Ubuntu EKS optimized AMI: https://cloud-images.ubuntu.com/aws-eks/
+node_ami            = "ami-03de35fda144b3672"


we should replace this with a data resource.

I personally prefer binding the AMI. If we query the AMI, the node group is going to recreated each time there's a new AMI. That can affect running tests. We might want to be able to do run terraform apply without actually having to rotate nodes.

I don't have strong opinion here. In case we want to keep this AMI hardcoded we need to think about its update schedule. AWS releases new AMIs pretty often.

ameukam · 2023-04-03T21:31:03Z

Thank you for working on this!

My understanding is that we'll use the same AWS account for prod and canary clusters, which I'm not sure is the desired state. I believe that account isolation can help us reduce security risks associated with access.

xmudrii · 2023-04-03T21:39:41Z

@ameukam I can see with @hh about getting a new account.

xmudrii

/lgtm
/approve
/hold
for @ameukam to take a look

k8s-ci-robot · 2023-04-04T10:24:23Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pkprzekwas, xmudrii

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~infra/aws/terraform/prow-build-cluster/OWNERS~~ [pkprzekwas,xmudrii]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ameukam · 2023-04-04T10:26:57Z

/lgtm

xmudrii · 2023-04-04T10:28:16Z

/hold cancel

pkprzekwas added 3 commits March 29, 2023 15:34

prow-build-canary-cluster: add scripts for provisioning canary eks cl…

516f914

…uster

TODO: fix prow cluster IAM

33b9725

add condition for assuming role, create manual

f3b5e10

k8s-ci-robot assigned ameukam and xmudrii Mar 31, 2023

k8s-ci-robot requested review from dims and thockin March 31, 2023 16:17

k8s-ci-robot assigned pkprzekwas Apr 3, 2023

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 3, 2023

pkprzekwas added 2 commits April 3, 2023 10:35

Merge remote-tracking branch 'origin/main' into prow-build-canary-clu…

ed09498

…ster

Merge canary and prod eks scripts into one

f8d56fe

k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Apr 3, 2023

pkprzekwas added 3 commits April 3, 2023 13:05

Update license

8de4842

Push tfvars

3d3847a

Fix prow role listing issue

7dcddaa

xmudrii reviewed Apr 3, 2023

View reviewed changes

pkprzekwas added 3 commits April 3, 2023 14:16

Address review comments

ad63575

Update readme with cluster usage manual

8ceaf98

Replace tf backends with TF workspaces

1949510

pkprzekwas requested a review from xmudrii April 3, 2023 14:05

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 3, 2023

xmudrii reviewed Apr 3, 2023

View reviewed changes

infra/aws/terraform/prow-build-cluster/kubernetes.tf Show resolved Hide resolved

infra/aws/terraform/prow-build-cluster/terraform.canary.tfvars Outdated Show resolved Hide resolved

xmudrii reviewed Apr 3, 2023

View reviewed changes

infra/aws/terraform/prow-build-cluster/Makefile Outdated Show resolved Hide resolved

ameukam reviewed Apr 3, 2023

View reviewed changes

pkprzekwas added 2 commits April 4, 2023 10:53

Change argument order in Makefile to fix assume_role & typo fix

0088b5d

Run tf fmt

a42cf0e

xmudrii approved these changes Apr 4, 2023

View reviewed changes

k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Apr 4, 2023

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 4, 2023

k8s-ci-robot merged commit 4622846 into kubernetes:main Apr 4, 2023

k8s-ci-robot added this to the v1.28 milestone Apr 4, 2023

pkprzekwas deleted the prow-build-canary-cluster branch July 6, 2023 15:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

prow-build-canary-cluster: provisioning scripts #5063

prow-build-canary-cluster: provisioning scripts #5063

pkprzekwas commented Mar 31, 2023 •

edited

Loading

pkprzekwas commented Apr 3, 2023

pkprzekwas commented Apr 3, 2023

pkprzekwas commented Apr 3, 2023

xmudrii left a comment

ameukam Apr 3, 2023

ameukam Apr 3, 2023

pkprzekwas Apr 4, 2023

ameukam Apr 3, 2023

xmudrii Apr 3, 2023

ameukam Apr 3, 2023

xmudrii Apr 3, 2023

pkprzekwas Apr 4, 2023

ameukam commented Apr 3, 2023

xmudrii commented Apr 3, 2023

xmudrii left a comment

k8s-ci-robot commented Apr 4, 2023

ameukam commented Apr 4, 2023

xmudrii commented Apr 4, 2023

	canary_prefix = terraform.workspace != "prod" ? "canary-" : ""
	canary_prefix = terraform.workspace != "prod-" ? "canary-" : ""

prow-build-canary-cluster: provisioning scripts #5063

prow-build-canary-cluster: provisioning scripts #5063

Conversation

pkprzekwas commented Mar 31, 2023 • edited Loading

pkprzekwas commented Apr 3, 2023

pkprzekwas commented Apr 3, 2023

pkprzekwas commented Apr 3, 2023

xmudrii left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ameukam commented Apr 3, 2023

xmudrii commented Apr 3, 2023

xmudrii left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Apr 4, 2023

ameukam commented Apr 4, 2023

xmudrii commented Apr 4, 2023

pkprzekwas commented Mar 31, 2023 •

edited

Loading