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-1597] e2e tests: Recover assigned pods in Bound state #544

Closed
wants to merge 4 commits into from

Conversation

pbacsko
Copy link
Contributor

@pbacsko pbacsko commented Mar 1, 2023

What is this PR for?

The PR contains an e2e test which validates that YK can recover properly with Running/Pending placeholders.

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-1597

How should this be tested?

Screenshots (if appropriate)

Questions:

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

@pbacsko pbacsko self-assigned this Mar 1, 2023
@pbacsko pbacsko force-pushed the YUNIKORN-1597-E2E branch 3 times, most recently from aabe70a to 3c6aca2 Compare March 1, 2023 13:39
@pbacsko pbacsko requested review from manirajv06 and wilfred-s and removed request for manirajv06 March 1, 2023 18:16
@craigcondit
Copy link
Contributor

craigcondit commented Mar 6, 2023

I'm -1 on this approach as it doesn't allow for running on anything but a Kind cluster. Our tests should be cluster-agnostic, and adapt to the environment they are run in so that they can be used on real clusters as well.

@craigcondit
Copy link
Contributor

craigcondit commented Mar 6, 2023

Alternate approach: Query the cluster for nodes with available capacity, and construct pods with nodeAffinity specified so that they are only schedulable on those nodes.

@pbacsko
Copy link
Contributor Author

pbacsko commented Mar 6, 2023

I'm -1 on this approach as it doesn't allow for running on anything but a Kind cluster. Our tests should be cluster-agnostic, and adapt to the environment they are run in so that they can be used on real clusters as well.

Yeah, most tests are agnostic, but there are some (maybe just the one which I modified) which already depends on having 3 nodes (1 control + 2 worker). Perhaps the existing tests should be fixed, too in a follow-up JIRA.

Alternate approach: Query the cluster for nodes with available capacity, and construct pods with nodeAffinity specified so that they are only schedulable on those nodes.

Hm, interesting. I'll try this.

@pbacsko pbacsko force-pushed the YUNIKORN-1597-E2E branch 2 times, most recently from 3c6dc6c to 17d1d80 Compare March 8, 2023 18:01
@pbacsko pbacsko requested a review from craigcondit March 8, 2023 18:04
@codecov
Copy link

codecov bot commented Mar 8, 2023

Codecov Report

Merging #544 (30beb9b) into master (20116e5) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #544   +/-   ##
=======================================
  Coverage   69.97%   69.97%           
=======================================
  Files          46       46           
  Lines        7881     7881           
=======================================
  Hits         5515     5515           
  Misses       2163     2163           
  Partials      203      203           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@pbacsko
Copy link
Contributor Author

pbacsko commented Mar 9, 2023

Test failure seems to be unrelated.

@manirajv06
Copy link
Contributor

@pbacsko PR has introduced new methods to set/remove node labels etc. There was a need to add taint/untaint node methods as well to remove the dependency on k8s test framework. Hence used some methods from this PR to above said PR. So, Please rebase your wc and take it forward.

@pbacsko pbacsko force-pushed the YUNIKORN-1597-E2E branch 4 times, most recently from 1bc0299 to d7a6c55 Compare March 31, 2023 13:59
Copy link
Contributor

@craigcondit craigcondit left a comment

Choose a reason for hiding this comment

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

+1 LGTM.

@pbacsko pbacsko deleted the YUNIKORN-1597-E2E branch April 28, 2024 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants