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

Update test case for Attached Cluster and fix link in doc #980

Merged
merged 2 commits into from
Nov 1, 2023

Conversation

gemmahou
Copy link
Collaborator

@gemmahou gemmahou commented Oct 30, 2023

Change description

  1. Update test case with test EKS cluster k8s version 1.27(The highest version supported by Multiclould API). AWS will stop supporting version 1.25 on May 2024. After this PR checked in, we should consider remove test version 1.25.

  2. Fixed the link in our documentation.

Fixes b/307542957

Tests you have done

go test -v -tags=integration ./pkg/controller/dynamic/ -test.run TestCreateNoChangeUpdateDelete -run-tests containerattached -timeout 900s

Dynamic tests passed locally: https://screenshot.googleplex.com/48XxECZ99kcXTKU.png

  • Run make ready-pr to ensure this PR is ready for review.
  • Perform necessary E2E testing for changed resources.

@maqiuyujoyce
Copy link
Collaborator

Thank you for the change, @gemmahou ! Though I am curious - why don't we replace the existing test case with this one? Is there any reason we want to keep the 1.25 one?

@gemmahou
Copy link
Collaborator Author

gemmahou commented Nov 1, 2023

Thank you for the change, @gemmahou ! Though I am curious - why don't we replace the existing test case with this one? Is there any reason we want to keep the 1.25 one?

The 1.25 one can be removed. But I want to hold until new test submitted, to avoid potential test failures in the Prow job. Maybe it's fine to remove the old test here as the local test passed.

@maqiuyujoyce
Copy link
Collaborator

Yes, I believe it's totally fine to just replace it. We can always make further changes if needed.

Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gemmahou, maqiuyujoyce

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:
  • OWNERS [gemmahou,maqiuyujoyce]

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

@gemmahou
Copy link
Collaborator Author

gemmahou commented Nov 1, 2023

Yes, I believe it's totally fine to just replace it. We can always make further changes if needed.

Updated the existing test.
Note that I created a new test EKS cluster to be used for testing, instead of upgrading the existing one. The reason is, we are still running the old test configuration. I'm afraid if I upgrade the existing EKS cluster directly, current test would fail until this PR takes effect.

@gemmahou gemmahou changed the title Add one more test case for Attached Cluster and fix link in doc Update test case for Attached Cluster and fix link in doc Nov 1, 2023
@google-oss-prow google-oss-prow bot merged commit 7ea6ab6 into GoogleCloudPlatform:master Nov 1, 2023
4 checks passed
@gemmahou gemmahou deleted the 307542957 branch November 1, 2023 22:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants