Skip to content

remove k8s version pin to 1.25 for MEC e2e test#4885

Closed
nojnhuh wants to merge 1 commit into
kubernetes-sigs:mainfrom
nojnhuh:mec
Closed

remove k8s version pin to 1.25 for MEC e2e test#4885
nojnhuh wants to merge 1 commit into
kubernetes-sigs:mainfrom
nojnhuh:mec

Conversation

@nojnhuh
Copy link
Copy Markdown
Contributor

@nojnhuh nojnhuh commented May 28, 2024

What type of PR is this?

/kind failing-test

What this PR does / why we need it:

The edgezone e2e test is failing, and I thought I saw somewhere that it's because the v1.25.0 image that this was pinned to before no longer exists but now I can't seem to find evidence of that. Regardless, v1.25 is EOL so we shouldn't be pinning to it anymore. I couldn't find any commentary on #2836 as to the reason for the 1.25 pin originally.

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

Special notes for your reviewer:

  • cherry-pick candidate <-- the test is also failing on the release branches

TODOs:

  • squashed commits
  • includes documentation
  • adds unit tests

Release note:

NONE

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. kind/failing-test Categorizes issue or PR as related to a consistently or frequently failing test. labels May 28, 2024
@k8s-ci-robot
Copy link
Copy Markdown
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 nojnhuh. For more information see the Kubernetes Code Review Process.

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

Details 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 cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 28, 2024
@nojnhuh
Copy link
Copy Markdown
Contributor Author

nojnhuh commented May 28, 2024

/cherry-pick release-1.14
/cherry-pick release-1.15

/test pull-cluster-api-provider-azure-e2e-optional
/hold for optional tests

@k8s-infra-cherrypick-robot
Copy link
Copy Markdown

@nojnhuh: once the present PR merges, I will cherry-pick it on top of release-1.14 in a new PR and assign it to you.

Details

In response to this:

/cherry-pick release-1.14
/cherry-pick release-1.15

/test pull-cluster-api-provider-azure-e2e-optional
/hold for optional tests

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 the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 28, 2024
@codecov
Copy link
Copy Markdown

codecov Bot commented May 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 62.04%. Comparing base (4b436c1) to head (6fddb70).
Report is 6 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4885   +/-   ##
=======================================
  Coverage   62.04%   62.04%           
=======================================
  Files         201      201           
  Lines       16878    16878           
=======================================
  Hits        10472    10472           
  Misses       5623     5623           
  Partials      783      783           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Copy Markdown
Contributor

@mboersma mboersma left a comment

Choose a reason for hiding this comment

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

/lgtm

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

LGTM label has been added.

DetailsGit tree hash: d659d25b7ef67d088667c14926a78e688ff46cad

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

@nojnhuh: 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-azure-e2e-optional 6fddb70 link false /test pull-cluster-api-provider-azure-e2e-optional

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.

Details

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.

@nojnhuh
Copy link
Copy Markdown
Contributor Author

nojnhuh commented May 28, 2024

The platform image 'cncf-upstream:capi:ubuntu-2204-gen1:127.14.20240517' is not available. Verify that all fields in the storage profile are correct. For more details about storage profile information, please refer to https://aka.ms/storageprofile

@mboersma It looks like this is complaining that the image doesn't exist specifically within this EdgeZone. I wonder if that's why the k8s version was originally pinned. Is there an easy way to copy the existing images to that particular region?

I see that this command works from the CI sub:

az vm image show --location canadacentral --urn cncf-upstream:capi:ubuntu-2204-gen1:127.14.20240517

But this one which includes the EdgeZone doesn't:

az vm image show --location canadacentral --urn cncf-upstream:capi:ubuntu-2204-gen1:microsoftvancouver1:127.14.20240517

@mboersma
Copy link
Copy Markdown
Contributor

Is there an easy way to copy the existing images to that particular region?

Not that I know of. When we publish to the Azure Marketplace, it takes care of "global" replication, which seems to mean availability in all public regions. There aren't any options in the Marketplace UI itself that allow configuration of EdgeZones or any other replication option. (I agree it seems likely this is why the test pinned the image.) I'll do some searching and see if I can find anything.

@mboersma
Copy link
Copy Markdown
Contributor

Based on this comment I'm inferring that some out-of-band process had to happen to make that particular image available in the testing EdgeZone.

@upxinxin is that correct? Do you know how we might make a newer CAPZ image available to this e2e test introduced in #2836?

(BTW, I checked, and the 1.25.x images were all deprecated programmatically on May 25.)

replicas: ${WORKER_MACHINE_COUNT:=2}
selector: {}
selector:
matchLabels: null
Copy link
Copy Markdown
Contributor

@willie-yao willie-yao May 28, 2024

Choose a reason for hiding this comment

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

Just wanted to clarify: Why is this change needed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's what happened to get generated ¯\_(ツ)_/¯

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

That's interesting.. I wonder what change prompted that 🤔

@mboersma
Copy link
Copy Markdown
Contributor

mboersma commented May 29, 2024

@upxinxin @mogliang the Kubernetes 1.25 image this test required has been deprecated. Do you know how we can make a more recent CAPZ image available in the right Edge Zone so this test passes again?

@mboersma mboersma removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 4, 2024
@mboersma mboersma mentioned this pull request Jun 5, 2024
4 tasks
@garogarabedyan
Copy link
Copy Markdown

Please disable this e2e test.

@nojnhuh
Copy link
Copy Markdown
Contributor Author

nojnhuh commented Jun 5, 2024

Please disable this e2e test.

#4900

@jackfrancis
Copy link
Copy Markdown
Contributor

looks like we can close this PR?

@nojnhuh
Copy link
Copy Markdown
Contributor Author

nojnhuh commented Jun 5, 2024

@garogarabedyan Can we keep this test as long as we migrate off of the microsoftvancouver1 edge zone onto a different one?

@nojnhuh
Copy link
Copy Markdown
Contributor Author

nojnhuh commented Jun 5, 2024

@jackfrancis In order to run this test going forward, we need to at least change the edge zone that the test is running in and also update the VM image used once we make it available in that new region. I think we can do those things in this PR yet.

@garogarabedyan
Copy link
Copy Markdown

@garogarabedyan Can we keep this test as long as we migrate off of the microsoftvancouver1 edge zone onto a different one?

I think you can keep the source code of the test. I think you should not spend time to fix the test before disabling it.

@nojnhuh
Copy link
Copy Markdown
Contributor Author

nojnhuh commented Jun 10, 2024

Closing until we can reenable the test.

/close

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

@nojnhuh: Closed this PR.

Details

In response to this:

Closing until we can reenable the test.

/close

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.

@nojnhuh nojnhuh deleted the mec branch June 10, 2024 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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. kind/failing-test Categorizes issue or PR as related to a consistently or frequently failing test. release-note-none Denotes a PR that doesn't merit a release note. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

7 participants