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.

Sign up for GitHub

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

Jump to bottom

✨refactor: update to use new community ami account #5133

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

✨refactor: update to use new community ami account #5133

wants to merge 3 commits into from
+1,008 −240

Conversation

richardcase
Copy link
Member

@richardcase richardcase commented Oct 4, 2024 •
edited
Loading

What type of PR is this?

/kind bug

What this PR does / why we need it:

Updates to the project to start using the new CNCF owned AWS account for publishing AMIs. The code has had the old VMWare account removed and the new account ID added. Additionally the docs have been updated including clarifying that the public AMIs should only be used for non-production usage.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Relates #5131
Relates #4984

Special notes for your reviewer:

A new GitHub Actions workflow (see #5138 ) can be invoked to publish new images.

Checklist:

  • squashed commits
  • includes documentation
  • includes emojis
  • adds unit tests
  • adds or updates e2e tests

Release note:

action required
A new community owned AWS account is now being used for publishing AMIs. The old account owner by a company is no longer accessible and the images are being automatically deleted by AWS. Support for ubuntu 18.04 & 20.04 has been dropped and ubuntu 24.04 is now the default OS if no OS is supplied.

@k8s-ci-robot k8s-ci-robot added release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. kind/bug Categorizes issue or PR as related to a bug. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-priority size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Oct 4, 2024
@richardcase richardcase changed the title ✨WIP: refactor: update to use new community ami account WIP: ✨refactor: update to use new community ami account Oct 4, 2024
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 4, 2024
@richardcase
Copy link
Member Author

/test pull-cluster-api-provider-aws-test

nrb
nrb approved these changes Oct 4, 2024
View reviewed changes
Copy link
Contributor

@nrb nrb left a comment

Choose a reason for hiding this comment

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

/lgtm

Looks good, one question on supported versions but it's not a blocker by any means.

docs/book/src/development/amis.md Outdated
6. Build the AMIs using:

```shell
make build-ami-ubuntu-2004
Copy link
Contributor

Choose a reason for hiding this comment

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

How long should we support 20.04?

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 4, 2024
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 7, 2024
@k8s-ci-robot
Copy link
Contributor

New changes are detected. LGTM label has been removed.

@richardcase
Copy link
Member Author

Until #5138 merges and AMIs are published:

/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 Oct 8, 2024
@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Oct 8, 2024
@richardcase richardcase changed the title WIP: ✨refactor: update to use new community ami account ✨refactor: update to use new community ami account Oct 8, 2024
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 8, 2024
@richardcase
Copy link
Member Author

/unhold

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 14, 2024
@richardcase
Copy link
Member Author

/test ?

@k8s-ci-robot
Copy link
Contributor

@richardcase: The following commands are available to trigger required jobs:

  • /test pull-cluster-api-provider-aws-build
  • /test pull-cluster-api-provider-aws-build-docker
  • /test pull-cluster-api-provider-aws-test
  • /test pull-cluster-api-provider-aws-verify

The following commands are available to trigger optional jobs:

  • /test pull-cluster-api-provider-aws-apidiff-main
  • /test pull-cluster-api-provider-aws-e2e
  • /test pull-cluster-api-provider-aws-e2e-blocking
  • /test pull-cluster-api-provider-aws-e2e-clusterclass
  • /test pull-cluster-api-provider-aws-e2e-conformance
  • /test pull-cluster-api-provider-aws-e2e-conformance-with-ci-artifacts
  • /test pull-cluster-api-provider-aws-e2e-eks
  • /test pull-cluster-api-provider-aws-e2e-eks-gc
  • /test pull-cluster-api-provider-aws-e2e-eks-testing

Use /test all to run the following jobs that were automatically triggered:

  • pull-cluster-api-provider-aws-apidiff-main
  • pull-cluster-api-provider-aws-build
  • pull-cluster-api-provider-aws-build-docker
  • pull-cluster-api-provider-aws-test
  • pull-cluster-api-provider-aws-verify

In response to this:

/test ?

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@richardcase
Copy link
Member Author

I'm hoping the e2e will pass now 🤞

/test pull-cluster-api-provider-aws-e2e

@richardcase
Copy link
Member Author

/test pull-cluster-api-provider-aws-e2e

@damdo
Copy link
Member

damdo commented Oct 16, 2024

/test pull-cluster-api-provider-aws-e2e

@richardcase
Copy link
Member Author

I have a suspicion as to why its failing some of the tests. Will try and work on it this afternoon (after a rebase )

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 16, 2024
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 16, 2024
AndiDog
AndiDog reviewed Oct 16, 2024
View reviewed changes
docs/book/src/topics/images/built-amis.md Outdated Show resolved Hide resolved
docs/book/src/topics/images/built-amis.md Outdated Show resolved Hide resolved
@AndiDog AndiDog self-assigned this Oct 16, 2024
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from andidog. 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

@richardcase
Copy link
Member Author

/test pull-cluster-api-provider-aws-e2e

1 similar comment
@richardcase
Copy link
Member Author

/test pull-cluster-api-provider-aws-e2e

@richardcase
Copy link
Member Author

Getting the e2e tests passing after the AMI change is now blocked on #5165

@damdo
Copy link
Member

damdo commented Oct 19, 2024

/test pull-cluster-api-provider-aws-e2e

@richardcase
refactor: update to use new community ami account
9133e58 
Updates to the project to start using the new CNCF owned AWS account
for publishing AMIs. The code has had the old VMWare account removed
and the new account ID added. Additionally the docs have been updated
including clarifying that the public AMIs should only be used for
non-production usage.

Signed-off-by: Richard Case <richard.case@outlook.com>
@richardcase
docs: updates the docs and e2e
e1bc243 
This make changes to the developer and user documentation. And also
updates the k8s version used in the e2e test to a version where we have
an AMI available.

Signed-off-by: Richard Case <richard.case@outlook.com>
@richardcase
Copy link
Member Author

/test pull-cluster-api-provider-aws-e2e

@richardcase
Copy link
Member Author

/test pull-cluster-api-provider-aws-e2e

@richardcase
Copy link
Member Author

Seeing if it was a flake.

/test pull-cluster-api-provider-aws-e2e

@damdo
Copy link
Member

damdo commented Oct 19, 2024 •
edited
Loading

/test pull-cluster-api-provider-aws-e2e

@damdo
Copy link
Member

damdo commented Oct 19, 2024 

  [FAILED] Failed to create kind cluster "test-98a3ki": failed to pull image "kindest/node:v1.29.9": command "docker pull kindest/node:v1.29.9" failed with error: exit status 1

The most recent kindest/node available is v1.29.8 https://hub.docker.com/layers/kindest/node/v1.29.8/images/sha256-b69a150f9951ef41158ec76de381a920df2be3582fd16fc19cf4757eef0dded9?context=explore

@richardcase
Copy link
Member Author

Thanks @damdo. I will update and try again.

@richardcase
Copy link
Member Author

/test pull-cluster-api-provider-aws-e2e

1 similar comment
@richardcase
Copy link
Member Author

/test pull-cluster-api-provider-aws-e2e

@k8s-ci-robot
Copy link
Contributor

@richardcase: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-cluster-api-provider-aws-e2e a0c9b15 link false /test pull-cluster-api-provider-aws-e2e

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@richardcase
feat: update e2e test config and docs changes
aa4bcdc 
This updates the kubernetes versions used in the e2e tests to use k8s
versions that we AMIs for. It also marks some of the old CSI tests as
pending.

A few minor docs updates have been made as a result of review.

Signed-off-by: Richard Case <richard.case@outlook.com>
@richardcase
Copy link
Member Author

/test pull-cluster-api-provider-aws-e2e

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AndiDog AndiDog AndiDog left review comments

@nrb nrb nrb approved these changes

@Ankitasw Ankitasw Awaiting requested review from Ankitasw

@killianmuldoon killianmuldoon Awaiting requested review from killianmuldoon

Assignees

@nrb nrb

@AndiDog AndiDog

Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. needs-priority release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@richardcase @k8s-ci-robot @damdo @nrb @AndiDog