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

[YUNIKORN-2815] Run e2e tests against Kubernetes 1.31 #894

Closed
wants to merge 1 commit into from

Conversation

craigcondit
Copy link
Contributor

What is this PR for?

Update the ee2e test matrix to include Kubernetes 1.31. Also, set the InPlacePodVerticalScaling feature flag when running on Kind v1.27.x or later to facilitate easier testing of pod resizing.

What type of PR is it?

  • - Bug Fix
  • - Improvement
  • - Feature
  • - Documentation
  • - Hot Fix
  • - Refactoring

Todos

  • - Task

What is the Jira issue?

https://issues.apache.org/jira/browse/YUNIKORN-2815

How should this be tested?

Screenshots (if appropriate)

Questions:

  • - The licenses files need update.
  • - There is breaking changes for older versions.
  • - It needs documentation.

@craigcondit craigcondit self-assigned this Aug 16, 2024
Update the ee2e test matrix to include Kubernetes 1.31. Also, set the
InPlacePodVerticalScaling feature flag when running on Kind v1.27.x or
later to facilitate easier testing of pod resizing.
Copy link

codecov bot commented Aug 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.81%. Comparing base (2278b32) to head (31ae008).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #894      +/-   ##
==========================================
- Coverage   67.84%   67.81%   -0.03%     
==========================================
  Files          70       70              
  Lines        7485     7485              
==========================================
- Hits         5078     5076       -2     
- Misses       2195     2196       +1     
- Partials      212      213       +1     

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

@craigcondit
Copy link
Contributor Author

Test failures are all preemption - unrelated to this PR. All other e2e tests passed.

Copy link
Contributor

@wilfred-s wilfred-s left a comment

Choose a reason for hiding this comment

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

VPA was pushed out late in the 1.31 release cycle. The list of fixes required to move to beta is big and some are really basic. There are a couple of really basic fixes that are only in 1.30 or 1.31.
I think we only ever want to test VPA with 1.31, maybe 1.30, and later. Before that too many things are broken.

@craigcondit
Copy link
Contributor Author

craigcondit commented Aug 19, 2024

I enabled the feature for 1.27 and later because that's when it was first implemented. The test we need to perform to validate that YuniKorn is behaving properly is simply to resize a pod and verify that the resource utilization within YuniKorn itself has been updated. That should be possible to test even as old as 1.27. Adding support within YuniKorn also doesn't affect older versions unless someone has actually turned the feature flag on.

@wilfred-s
Copy link
Contributor

One of the basic problem described upstream is linked to testing. E2e testing of the feature has shown really bad performance when trying to use VPA. Changes taking the kubelet sync time, 60 seconds, to propagate and get actioned. Changes like these should always take a 1 second or so, but code is not working correctly.
Container spec defaulting was also changed between releases which means we might see fields in 1.31 set by default which we do not see in 1.27 or the other way around.

We just need to be careful that we're not adding tests that are flaky or can only work on the latest release etc.

@craigcondit
Copy link
Contributor Author

craigcondit commented Aug 19, 2024

Differences in various versions are precisely why we want to enable the flag for the widest possible cross-section of releases. Because users can turn this on, there's a possibility that they will. If we do find that there's a test that only works on newer versions, we can disable that test for older releases. However, we will not know this until we test on the widest possible range of releases.

Copy link
Contributor

@zhuqi-lucas zhuqi-lucas left a comment

Choose a reason for hiding this comment

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

LGTM

@craigcondit craigcondit deleted the YUNIKORN-2815 branch August 19, 2024 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants