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

Nodeaffinity vk #486

Merged
merged 6 commits into from Apr 23, 2021
Merged

Nodeaffinity vk #486

merged 6 commits into from Apr 23, 2021

Conversation

nilekhc
Copy link
Contributor

@nilekhc nilekhc commented Apr 23, 2021

Reason for Change:

Change adds nodeaffinity to prevent scheduling on virtual-kubelet nodes.

Requirements

  • squashed commits
  • included documentation
  • added unit tests and e2e tests (if applicable).

Issue Fixed:

#472

Does this change contain code from or inspired by another project?

  • Yes
  • No

If "Yes," did you notify that project's maintainers and provide attribution?

Special Notes for Reviewers:
Should we create new patch version(0.0.19) for chart?

Copy link
Member

@aramase aramase left a comment

Choose a reason for hiding this comment

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

Thank you for looking into this. The changes look good, just one small nit:

  1. For chart or deployment yaml updates we update the yamls in the manifest_staging/ folder, where we host the staging charts and deployment yamls. All the yaml changes will then be promoted into the released charts folder with the next release.
  2. Also could you add the new configurable values to the configuration table.

@codecov-commenter
Copy link

codecov-commenter commented Apr 23, 2021

Codecov Report

Merging #486 (deb09ac) into master (5fed2b9) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #486   +/-   ##
=======================================
  Coverage   64.31%   64.31%           
=======================================
  Files           7        7           
  Lines         510      510           
=======================================
  Hits          328      328           
  Misses        148      148           
  Partials       34       34           

@nilekhc
Copy link
Contributor Author

nilekhc commented Apr 23, 2021

Thanks for the review @aramase . I have addressed above comments. Also added node affinity to deployment yamls.

Copy link
Member

@aramase aramase left a comment

Choose a reason for hiding this comment

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

lgtm

@aramase aramase merged commit dead813 into Azure:master Apr 23, 2021
@nilekhc nilekhc deleted the nodeaffinity-vk branch August 17, 2022 21:15
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.

None yet

3 participants