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

add terraform code to deploy patch release notification service #6853

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

cpanato
Copy link
Member

@cpanato cpanato commented May 30, 2024

this PR adds the terraform code to deploy the service we are building in kubernetes/release#3627

/assign @xmudrii @saschagrunert @ameukam
cc @kubernetes/release-managers

xref: kubernetes/release#3627

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 30, 2024
@k8s-ci-robot k8s-ci-robot requested review from dims and thockin May 30, 2024 11:50
@k8s-ci-robot k8s-ci-robot added the area/infra Infrastructure management, infrastructure design, code in infra/ label May 30, 2024
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: cpanato
Once this PR has been reviewed and has the lgtm label, please assign spiffxp for approval. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files:

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

@k8s-ci-robot k8s-ci-robot added area/infra/aws Issues or PRs related to Kubernetes AWS infrastructure sig/k8s-infra Categorizes an issue or PR as relevant to SIG K8s Infra. labels May 30, 2024
Comment on lines +11 to +16
To deploy will require to have both repositories cloned:

- https://github.com/kubernetes/release/
- https://github.com/kubernetes/k8s.io

from https://github.com/kubernetes/k8s.io
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a big fan of doing this because I think it's going to be very hard to automate the Terraform execution in the future. @upodroid Maybe you can help here, I'm thinking out loud, but can we setup SA with IRSA in EKS build cluster that would allow pushing to this registry, and then we run a ProwJob that's going to use that SA to build the image and push it to the registry?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the main idea here is to use terraform to build the image when is needed and deploy the lambda, then everytime we run that and if there are changes in the code it will generate a new image

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but it is not hard to automate, in the automation we clone both repos, we do this in several places already for other things

@upodroid
Copy link
Member

Try to follow the approach that we do for archeio(registry.k8s.io). We build an image, publish it and then update the terraform code to use that revision.

@cpanato
Copy link
Member Author

cpanato commented May 30, 2024

okay! will refactor that

@cpanato
Copy link
Member Author

cpanato commented May 30, 2024

Try to follow the approach that we do for archeio(registry.k8s.io). We build an image, publish it and then update the terraform code to use that revision.

but there is any reason to do that? why not build on the fly? we also reduce a few steps and overhead

@xmudrii
Copy link
Member

xmudrii commented May 30, 2024

but there is any reason to do that? why not build on the fly? we also reduce a few steps and overhead

One thing that I'm worried about is that depending on the CI/CD solution/approach we go with for Terraform, it might be hard/impossible to also clone k/release and run the build process. Building also requires some more resources than just applying Terraform configs, so there's that too. This is kind of wild guess, but it's better to avoid such problems if we can.

@cpanato
Copy link
Member Author

cpanato commented May 30, 2024

but there is any reason to do that? why not build on the fly? we also reduce a few steps and overhead

One thing that I'm worried about is that depending on the CI/CD solution/approach we go with for Terraform, it might be hard/impossible to also clone k/release and run the build process. Building also requires some more resources than just applying Terraform configs, so there's that too. This is kind of wild guess, but it's better to avoid such problems if we can.

I would disagree with that, but I will work on refactoring in the way k8s infra likes more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also commit terraform.tfvars (if no sensitive information) or add default values to this file.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will commit, but as we discussed here, I will need to refactor not to build the image here and use a prebuilt image that will be done in another job.

but, I need to know how to push the image to the AWS account and more specifically to the aws account we are interested in, how that can be done? cc @upodroid

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cpanato I would try the following approach if it's okay with @upodroid:

  • Create a new IAM role that can push to the said registry
  • Create a new ServiceAccount inside the EKS Prow build cluster
  • Bind SA and IAM role via IRSA
  • Create a ProwJob that utilizes the said SA and make it build and push the image

There might some additional steps between 1 and 2 in terms of configuring the AWS account, but we have done that before.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If y'all are okay with that, I can take care of setting up everything because I think I have access to all the resources/accounts that we need for this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we will build it using cloudbuild? or can i use ko (https://github.com/ko-build/ko) to do the image/push build?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should be able to use ko, let me take a look.


terraform {
backend "s3" {
bucket = "tf-state-sig-release"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this bucket manually created?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, we need to bootstrap some how :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I have been doing before for both SIG Release and SIG K8s Infra is using CloudFormation: https://github.com/kubernetes/k8s.io/blob/main/infra/aws/cloudformation/iam/cdn.packages.k8s.io/cloudformation.yaml

Maybe we can take the same approach here if that's okay for y'all. It still requires manual bootstrapping (i.e. running that CloudFormation script), but all the resources are created automatically.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yikes! cloudformation, i thought that did not exist anymore LOL

but sounds good to me, i can add that, and delete the bucket that I've create before.
I will add that as well in this PR
thanks!

region = "us-west-2"
}

required_version = "1.8.0"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we relax the version if we don't have any dependency on 1.8.0?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure

@cpanato
Copy link
Member Author

cpanato commented May 31, 2024

/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 31, 2024
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

  • Mark this PR as fresh with /remove-lifecycle stale
  • Close this PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/infra/aws Issues or PRs related to Kubernetes AWS infrastructure area/infra Infrastructure management, infrastructure design, code in infra/ cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. sig/k8s-infra Categorizes an issue or PR as relevant to SIG K8s Infra. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants