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

feat: imagePullJob support Tolerations #1705

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

zerunhu
Copy link

@zerunhu zerunhu commented Aug 16, 2024

Ⅰ. Describe what this PR does

The ImagePullJob supports a tolerations field to tolerate node taints.

Ⅱ. Does this pull request fix one issue?

#1694

Ⅲ. Describe how to verify it

Ⅳ. Special notes for reviews

@kruise-bot
Copy link

Welcome @zerunhu! It looks like this is your first PR to openkruise/kruise 🎉

@kruise-bot kruise-bot added the size/L size/L: 100-499 label Aug 16, 2024
Copy link

codecov bot commented Aug 16, 2024

Codecov Report

Attention: Patch coverage is 67.85714% with 9 lines in your changes missing coverage. Please review.

Project coverage is 49.34%. Comparing base (0d0031a) to head (cff4e59).
Report is 91 commits behind head on master.

Files with missing lines Patch % Lines
pkg/util/imagejob/imagejob_reader.go 79.16% 4 Missing and 1 partial ⚠️
...controller/imagepulljob/imagepulljob_controller.go 0.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1705      +/-   ##
==========================================
+ Coverage   47.91%   49.34%   +1.42%     
==========================================
  Files         162      191      +29     
  Lines       23491    19636    -3855     
==========================================
- Hits        11256     9689    -1567     
+ Misses      11014     8688    -2326     
- Partials     1221     1259      +38     
Flag Coverage Δ
unittests 49.34% <67.85%> (+1.42%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@zerunhu
Copy link
Author

zerunhu commented Aug 16, 2024

While testing E2E, I found that a Node had a Taint added.

[
      {
          "key": "node-role.kubernetes.io/control-plane",
          "effect": "NoSchedule"
      }
]

Due to the Toleration feature of the imagePullJob, it won't be scheduled, causing the test to fail. I realized that I need to modify a large amount of code in test/e2e/apps/pullimages.go. I hope to get some help on how to make these modifications more effectively. @furykerry

@zmberg
Copy link
Member

zmberg commented Aug 28, 2024

/lgtm

@kruise-bot
Copy link

[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 zmberg by writing /assign @zmberg in a comment. For more information see:The Kubernetes Code Review Process.

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

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

@zerunhu zerunhu force-pushed the feat/ImagePullJob-support-Tolerations branch 3 times, most recently from c699156 to 8da250d Compare September 3, 2024 02:59
@kruise-bot kruise-bot added size/XXL size/L size/L: 100-499 and removed size/L size/L: 100-499 size/XXL labels Sep 3, 2024
@zerunhu zerunhu force-pushed the feat/ImagePullJob-support-Tolerations branch from 1d2a405 to af972e3 Compare September 3, 2024 03:34
@kruise-bot kruise-bot added size/XXL and removed size/L size/L: 100-499 labels Sep 3, 2024
@zerunhu zerunhu force-pushed the feat/ImagePullJob-support-Tolerations branch from af972e3 to b683b82 Compare September 3, 2024 03:39
@kruise-bot kruise-bot added size/L size/L: 100-499 and removed size/XXL labels Sep 3, 2024
@zerunhu zerunhu force-pushed the feat/ImagePullJob-support-Tolerations branch from b683b82 to a7f922f Compare September 3, 2024 03:43
@zerunhu
Copy link
Author

zerunhu commented Sep 3, 2024

The test has passed. Could you please help review this PR?
@furykerry @zmberg

@zmberg
Copy link
Member

zmberg commented Sep 3, 2024

/hold

@zerunhu zerunhu force-pushed the feat/ImagePullJob-support-Tolerations branch 2 times, most recently from 900433e to 014d925 Compare September 3, 2024 09:26
@zerunhu
Copy link
Author

zerunhu commented Sep 3, 2024

Hello, I have pulled your latest fix code, and all tests have passed now. Could you please help review the PR again? Since I am a beginner with Kruise, I hope you can provide me with some information on what I should do next. @zmberg

@zerunhu zerunhu force-pushed the feat/ImagePullJob-support-Tolerations branch 6 times, most recently from 5d3013f to e8eca6f Compare September 24, 2024 10:58
@zerunhu zerunhu force-pushed the feat/ImagePullJob-support-Tolerations branch from e8eca6f to cff4e59 Compare September 24, 2024 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants