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

DNM: e2e-eks: debug EKS invalid awsmanagedmachinepool spec.roleName #5179

Closed
wants to merge 1 commit into from
Closed

DNM: e2e-eks: debug EKS invalid awsmanagedmachinepool spec.roleName #5179

wants to merge 1 commit into from

Conversation

damdo
Copy link
Member

@damdo damdo commented Oct 24, 2024

Debug EKS e2e issue:

[managed] [general] [ipv6] EKS cluster tests [It] should create a cluster and add nodes
/home/prow/go/src/sigs.k8s.io/cluster-api-provider-aws/test/e2e/shared/common.go:231
  [FAILED] Unexpected error:
      <errors.aggregate | len:1, cap:1>: 
      admission webhook "validation.awsmanagedmachinepool.infrastructure.cluster.x-k8s.io" denied the request: AWSManagedMachinePool.infrastructure.cluster.x-k8s.io "eks-nodes-14zug4-pool-0" is invalid: spec.roleName: Invalid value: "": field is immutable

https://prow.k8s.io/view/gs/kubernetes-ci-logs/pr-logs/pull/kubernetes-sigs_cluster-api-provider-aws/5093/pull-cluster-api-provider-aws-e2e-eks/1849186934245560320

/hold

@k8s-ci-robot
Copy link
Contributor

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

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.

@k8s-ci-robot k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-priority size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Oct 24, 2024
@k8s-ci-robot k8s-ci-robot requested review from Ankitasw and faiq October 24, 2024 07:20
@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 assign nrb 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

@damdo
Copy link
Member Author

damdo commented Oct 24, 2024

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

@damdo damdo changed the title DNM: e2e-eks: debug EKS invalid spec.Rolename DNM: e2e-eks: debug EKS invalid awsmanagedmachinepool spec.roleName Oct 24, 2024
@damdo damdo mentioned this pull request Oct 24, 2024
Merged
5 tasks
@damdo
Copy link
Member Author

damdo commented Oct 24, 2024

It looks like EKS E2Es are permafailing with https://prow.k8s.io/view/gs/kubernetes-ci-logs/pr-logs/pull/kubernetes-sigs_cluster-api-provider-aws/5179/pull-cluster-api-provider-aws-e2e-eks/1849350227388010496

/cc. @kubernetes-sigs/cluster-api-provider-aws-maintainers

@richardcase richardcase mentioned this pull request Oct 24, 2024
Closed
@richardcase
Copy link
Member

richardcase commented Oct 26, 2024 •
edited
Loading

@damdo - it would be worth rebase and running the eks e2e to check it solves the issue you were seeing.

@damdo damdo force-pushed the debug-eks-invalid-rolename branch from aa29737 to 61f82de Compare October 26, 2024 10:29
@damdo
Copy link
Member Author

damdo commented Oct 26, 2024

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

@damdo
Copy link
Member Author

damdo commented Oct 26, 2024

Cool done, let's see

@damdo
Copy link
Member Author

damdo commented Oct 26, 2024

/retest

1 similar comment
@damdo
Copy link
Member Author

damdo commented Oct 26, 2024

/retest

@richardcase
Copy link
Member

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

@richardcase
Copy link
Member

The main thing is that the EKS e2e are passing on this.

There is still some flakiness in the non-EKS e2e. So if that fails I wouldn't worry 😁

@richardcase
Copy link
Member

And thank you @damdo for the rebase and testing it again.

@damdo
Copy link
Member Author

damdo commented Oct 27, 2024

The main thing is that the EKS e2e are passing on this.
There is still some flakiness in the non-EKS e2e. So if that fails I wouldn't worry 😁

Yeah indeed. Ah yes I was trying to make sure they were only flakes. Makes sense to me, closing this as it served it purpose 👍

@damdo damdo closed this Oct 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Ankitasw Ankitasw Awaiting requested review from Ankitasw

@faiq faiq Awaiting requested review from faiq

Assignees
No one assigned
Labels
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. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. needs-priority size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

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