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 the image status when start pulling images #1280

Merged
merged 1 commit into from
May 11, 2023

Conversation

diannaowa
Copy link
Contributor

@diannaowa diannaowa commented May 8, 2023

Ⅰ. Describe what this PR does

We should update the image status when we start pulling images,
which can meet the scenario that some large size images cannot return the result from CRI.PullImage within 60s. For one reason:
1.For nodeimage controller will mark image:tag task failed (not responded for a long time) if daemon does not report status in 60s.

When checking whether a image exists, we should compare the name(without defaultDomain and officialRepoName) and tag of the image.

Ⅱ. Does this pull request fix one issue?

fixes #1273

Ⅲ. Describe how to verify it

Ⅳ. Special notes for reviews

@kruise-bot kruise-bot requested review from Fei-Guo and FillZpp May 8, 2023 10:29
@kruise-bot kruise-bot added the size/S size/S 10-29 label May 8, 2023
@kruise-bot kruise-bot added size/M size/M: 30-99 and removed size/S size/S 10-29 labels May 8, 2023
@diannaowa diannaowa force-pushed the fix_1273 branch 2 times, most recently from 33fb4dd to 0026f1a Compare May 8, 2023 14:17
@codecov-commenter
Copy link

codecov-commenter commented May 8, 2023

Codecov Report

Merging #1280 (8121c7e) into master (7f5046d) will increase coverage by 0.03%.
The diff coverage is 100.00%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##           master    #1280      +/-   ##
==========================================
+ Coverage   48.45%   48.48%   +0.03%     
==========================================
  Files         149      149              
  Lines       20627    20614      -13     
==========================================
  Hits         9994     9994              
+ Misses       9537     9523      -14     
- Partials     1096     1097       +1     
Flag Coverage Δ
unittests 48.48% <100.00%> (+0.03%) ⬆️

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

Impacted Files Coverage Δ
pkg/daemon/criruntime/imageruntime/helpers.go 20.79% <100.00%> (+9.38%) ⬆️

... and 1 file with indirect coverage changes

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

Copy link
Member

@zmberg zmberg left a comment

Choose a reason for hiding this comment

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

Please avoid changing the import section as much as possible.

@diannaowa
Copy link
Contributor Author

Please avoid changing the import section as much as possible.

got it, thanks for reminding

…defaultDomain and officialRepoName) name and tag of image

Signed-off-by: liuzhenwei <dui_zhang@163.com>

add ut for ContainsImage

Signed-off-by: liuzhenwei <dui_zhang@163.com>

revert import

Signed-off-by: liuzhenwei <dui_zhang@163.com>
@zmberg
Copy link
Member

zmberg commented May 10, 2023

/lgtm

@zmberg
Copy link
Member

zmberg commented May 11, 2023

/approve

@kruise-bot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: zmberg

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:

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

@kruise-bot kruise-bot merged commit ccd94b2 into openkruise:master May 11, 2023
diannaowa added a commit to diannaowa/kruise that referenced this pull request Jun 2, 2023
…defaultDomain and officialRepoName) name and tag of image (openkruise#1280)

add ut for ContainsImage



revert import

Signed-off-by: liuzhenwei <dui_zhang@163.com>
diannaowa added a commit to diannaowa/kruise that referenced this pull request Jun 2, 2023
…defaultDomain and officialRepoName) name and tag of image (openkruise#1280)

add ut for ContainsImage



revert import

Signed-off-by: liuzhenwei <dui_zhang@163.com>
lilongfeng0902 pushed a commit to lilongfeng0902/kruise that referenced this pull request Sep 12, 2023
…defaultDomain and officialRepoName) name and tag of image (openkruise#1280)

add ut for ContainsImage



revert import

Signed-off-by: liuzhenwei <dui_zhang@163.com>
ppbits pushed a commit to ppbits/kruise that referenced this pull request Apr 4, 2024
…defaultDomain and officialRepoName) name and tag of image (openkruise#1280)

add ut for ContainsImage



revert import
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.

[BUG] Image Pre-download failed in k8s 1.26
4 participants