Support predownload image in ads #1057

ABNER-1 · 2022-08-14T14:14:58Z

Ⅰ. Describe what this PR does

Support predownload image in ads

Ⅱ. Does this pull request fix one issue?

Ⅲ. Describe how to verify it

create ads and change images of it, then an imagepulljob will be created.
If every pods have beed updated, the imagepulljob will be deleted.

Ⅳ. Special notes for reviews

I am not sure whether it is neccessary to limit the daemonset size.
I am so sorry that I did not found how to make the predownload image order correspond to the order of daemonset update.

Signed-off-by: Abner <abner199709@gmail.com>

sonatype-lift · 2022-08-14T14:42:01Z

pkg/controller/daemonset/daemonset_predownload_image.go

+
+import (
+	"context"
+	"fmt"


goimports: File is not goimports-ed

Reply with "@sonatype-lift help" for info about LiftBot commands.
Reply with "@sonatype-lift ignore" to tell LiftBot to leave out the above finding from this PR.
Reply with "@sonatype-lift ignoreall" to tell LiftBot to leave out all the findings from this PR and from the status bar in Github.

When talking to LiftBot, you need to refresh the page to see its response. Click here to get to know more about LiftBot commands.

Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

@sonatype-lift help

I’m LiftBot and I automatically analyze new code in code review, and comment when I find potential bugs. I also accept comments as commands. Just @sonatype-lift followed by the command: ignore to mark as false positive, unignore to undo, ignoreall to ignore all issues in this PR, exclude <file|path|tool|issue> to discover config file snippets, and help to see this message. Click here to add LiftBot to another repo.

sonatype-lift · 2022-08-14T14:42:03Z

pkg/controller/daemonset/daemonset_controller.go

@@ -21,6 +21,10 @@ import (
 	"context"
 	"flag"
 	"fmt"
+	"github.com/openkruise/kruise/pkg/features"


goimports: File is not goimports-ed

Reply with "@sonatype-lift help" for info about LiftBot commands.
Reply with "@sonatype-lift ignore" to tell LiftBot to leave out the above finding from this PR.
Reply with "@sonatype-lift ignoreall" to tell LiftBot to leave out all the findings from this PR and from the status bar in Github.

When talking to LiftBot, you need to refresh the page to see its response. Click here to get to know more about LiftBot commands.

Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

FillZpp · 2022-08-16T03:03:13Z

@ABNER-1 The daemonset UT failed, could you fix it?

ABNER-1 · 2022-08-21T08:38:47Z

@ABNER-1 The daemonset UT failed, could you fix it?

The unittest init struct is complex and different from cloneset.
I am reading the implements.
I will fix it when I think I can get it.

Signed-off-by: Abner <abner199709@gmail.com>

ABNER-1 · 2022-08-24T11:43:03Z

Looks like these two pull_request test(other) failed due to statefulset controller.
However, I have no code changes about it. @FillZpp

pkg/controller/daemonset/daemonset_controller.go

Signed-off-by: Abner <abner199709@gmail.com>

codecov-commenter · 2022-09-08T15:31:29Z

Codecov Report

Merging #1057 (cf1c5c6) into master (4263a74) will increase coverage by 0.61%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #1057      +/-   ##
==========================================
+ Coverage   49.30%   49.92%   +0.61%     
==========================================
  Files         125      127       +2     
  Lines       12355    17961    +5606     
==========================================
+ Hits         6092     8967    +2875     
- Misses       5322     8005    +2683     
- Partials      941      989      +48

Flag	Coverage Δ
unittests	`49.92% <0.00%> (+0.61%)`	⬆️

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

Impacted Files	Coverage Δ
pkg/controller/daemonset/daemonset_controller.go	`40.96% <0.00%> (-1.73%)`	⬇️
...ontroller/daemonset/daemonset_predownload_image.go	`0.00% <0.00%> (ø)`
...oller/statefulset/statefulset_predownload_image.go	`0.00% <ø> (ø)`
...ontroller/advancedcronjob/advancedcronjob_utils.go	`75.00% <0.00%> (-25.00%)`	⬇️
pkg/util/cache.go	`53.84% <0.00%> (-6.16%)`	⬇️
...b/validating/broadcastjob_create_update_handler.go	`42.02% <0.00%> (-3.98%)`	⬇️
pkg/util/lock.go	`75.00% <0.00%> (-2.78%)`	⬇️
...ontroller/podreadiness/pod_readiness_controller.go	`28.78% <0.00%> (-2.59%)`	⬇️
pkg/controller/uniteddeployment/subset_control.go	`70.27% <0.00%> (-2.57%)`	⬇️
...g/webhook/pod/validating/pod_unavailable_budget.go	`61.79% <0.00%> (-2.39%)`	⬇️
... and 119 more

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

furykerry · 2022-09-13T08:14:22Z

/lgtm

furykerry · 2022-09-20T05:50:04Z

pkg/controller/daemonset/daemonset_controller.go

-
+	if !utildiscovery.DiscoverGVK(appsv1alpha1.SchemeGroupVersion.WithKind("ImagePullJob")) ||
+		!utilfeature.DefaultFeatureGate.Enabled(features.KruiseDaemon) ||
+		!utilfeature.DefaultFeatureGate.Enabled(features.PreDownloadImageForInPlaceUpdate) {


plz add a separate feature gate for PreDownloadImageForDaemonSet that is disabled by default, predownload for ads is still lack some feature e.g. updateReadyPods check before predownload

FillZpp · 2022-09-28T03:40:34Z

/approve

kruise-bot · 2022-09-28T03:47:54Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: FillZpp

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:

~~OWNERS~~ [FillZpp]

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

Signed-off-by: peng.xin <peng.xin@ly.com> optimize workloadspread when suitable subset maxReplicas is niil (openkruise#1066) Signed-off-by: mingzhou.swx <mingzhou.swx@alibaba-inc.com> Signed-off-by: mingzhou.swx <mingzhou.swx@alibaba-inc.com> Co-authored-by: mingzhou.swx <mingzhou.swx@alibaba-inc.com> Optimize performance of LabelSelector conversion (openkruise#1068) Signed-off-by: FillZpp <FillZpp.pub@gmail.com> Signed-off-by: FillZpp <FillZpp.pub@gmail.com> Support timezone for AdvancedCronJob (openkruise#1070) Signed-off-by: FillZpp <FillZpp.pub@gmail.com> Signed-off-by: FillZpp <FillZpp.pub@gmail.com> ignore if pod condition has been updated (openkruise#1074) Signed-off-by: mingzhou.swx <mingzhou.swx@alibaba-inc.com> Signed-off-by: mingzhou.swx <mingzhou.swx@alibaba-inc.com> Co-authored-by: mingzhou.swx <mingzhou.swx@alibaba-inc.com> pod probe marker apis (openkruise#1073) * pod probe marker apis Signed-off-by: liheng.zms <liheng.zms@alibaba-inc.com> * fix statefulset truncateHistory panic Signed-off-by: liheng.zms <liheng.zms@alibaba-inc.com> Signed-off-by: liheng.zms <liheng.zms@alibaba-inc.com> pod probe marker controller (openkruise#1075) Signed-off-by: liheng.zms <liheng.zms@alibaba-inc.com> Signed-off-by: liheng.zms <liheng.zms@alibaba-inc.com> consider whether patch field is matched when assign existing pods to subset (openkruise#1083) Signed-off-by: mingzhou.swx <mingzhou.swx@alibaba-inc.com> Signed-off-by: mingzhou.swx <mingzhou.swx@alibaba-inc.com> Co-authored-by: mingzhou.swx <mingzhou.swx@alibaba-inc.com> pod probe marker webhook (openkruise#1078) Signed-off-by: liheng.zms <liheng.zms@alibaba-inc.com> Signed-off-by: liheng.zms <liheng.zms@alibaba-inc.com> sidecarset support pods ns(kube-system, kube-public) (openkruise#1084) Signed-off-by: liheng.zms <liheng.zms@alibaba-inc.com> Signed-off-by: liheng.zms <liheng.zms@alibaba-inc.com> pod probe marker proposal (openkruise#1053) Signed-off-by: liheng.zms <liheng.zms@alibaba-inc.com> Signed-off-by: liheng.zms <liheng.zms@alibaba-inc.com> node pod probe daemon (openkruise#1077) Signed-off-by: liheng.zms <liheng.zms@alibaba-inc.com> sidecarset support patch pod metadata proposal (openkruise#993) Signed-off-by: liheng.zms <liheng.zms@alibaba-inc.com> Support predownload image in ads (openkruise#1057) Signed-off-by: Abner <abner199709@gmail.com> * support predownload image for ads Add PreDownloadImageForDaemonSetUpdate featureGate (openkruise#1093) Signed-off-by: FillZpp <FillZpp.pub@gmail.com> Signed-off-by: FillZpp <FillZpp.pub@gmail.com> Add changelog for v1.3.0 (openkruise#1092) Signed-off-by: liheng.zms <liheng.zms@alibaba-inc.com> Signed-off-by: liheng.zms <liheng.zms@alibaba-inc.com> fix sidecarset inject annotations abnormal (openkruise#1101) Signed-off-by: liheng.zms <liheng.zms@alibaba-inc.com> Signed-off-by: liheng.zms <liheng.zms@alibaba-inc.com> fix InPlaceUpdateEnvFromMetadata bug (openkruise#1108) Co-authored-by: 郭已钦 <xiangyang2@xiaohongshu.com> support uniteddeployment persistentVolumeClaimRetentionPolicy inherit from advancestatefulset template (openkruise#1110) Signed-off-by: cheyuexian <cheyuexian@bilibili.com> Signed-off-by: cheyuexian <cheyuexian@bilibili.com> Co-authored-by: cheyuexian <cheyuexian@bilibili.com> Add PreNormal Lifecycle Hook for CloneSet (openkruise#1071) * add pre-normal(pre-available) hook Signed-off-by: mingzhou.swx <mingzhou.swx@alibaba-inc.com> * add pre-normal(pre-available) hook Signed-off-by: mingzhou.swx <mingzhou.swx@alibaba-inc.com> Signed-off-by: mingzhou.swx <mingzhou.swx@alibaba-inc.com> Co-authored-by: mingzhou.swx <mingzhou.swx@alibaba-inc.com> partition support float percent (openkruise#1124) Signed-off-by: shiyan2016 <shiyan20160606@gmail.com> Signed-off-by: shiyan2016 <shiyan20160606@gmail.com> improve error hanlding of inplace update env from metadata (openkruise#1125) Signed-off-by: mingzhou.swx <mingzhou.swx@alibaba-inc.com> Signed-off-by: mingzhou.swx <mingzhou.swx@alibaba-inc.com> Co-authored-by: mingzhou.swx <mingzhou.swx@alibaba-inc.com> pub support to configure Evict,Delete,Update Operation (openkruise#1126) Signed-off-by: liheng.zms <liheng.zms@alibaba-inc.com> Signed-off-by: liheng.zms <liheng.zms@alibaba-inc.com> feat: add watch whitelist feat: add watch whitelist add watch whitelist check add dynamic watch option feat: add persistent annotations format feat: add pps support custom workload Signed-off-by: peng.xin <peng.xin@ly.com> fix: nodeSelector and nodeAffinity have not been modified Signed-off-by: peng.xin <peng.xin@ly.com> fix: error string Signed-off-by: peng.xin <peng.xin@ly.com> fix: error string Signed-off-by: peng.xin <peng.xin@ly.com> fix: goimports Signed-off-by: peng.xin <peng.xin@ly.com> refactor: refactoring ersistentPodAnnotations type Signed-off-by: peng.xin <peng.xin@ly.com> - add statefulet group check - modify the pod create processing logic Signed-off-by: peng.xin <peng.xin@ly.com> refactor: pps watch of whitelist workload Signed-off-by: peng.xin <peng.xin@ly.com> fix: add statefulsetlike's pod controller Signed-off-by: peng.xin <peng.xin@ly.com>

* support predownload image for ads

support predownload image for ads

b1c9c13

Signed-off-by: Abner <abner199709@gmail.com>

kruise-bot requested review from Fei-Guo and FillZpp August 14, 2022 14:15

kruise-bot added the size/L size/L: 100-499 label Aug 14, 2022

ABNER-1 force-pushed the support-predownload-image-in-ads branch from 4c8b400 to 0de6458 Compare August 14, 2022 14:22

test ads predownload image ok

4d553b3

Signed-off-by: Abner <abner199709@gmail.com>

ABNER-1 force-pushed the support-predownload-image-in-ads branch from 0de6458 to 4d553b3 Compare August 14, 2022 14:23

sonatype-lift bot reviewed Aug 14, 2022

View reviewed changes

fmt go import

c8dab13

Signed-off-by: Abner <abner199709@gmail.com>

ABNER-1 force-pushed the support-predownload-image-in-ads branch from b82fa6d to c8dab13 Compare August 23, 2022 14:29

furykerry reviewed Sep 5, 2022

View reviewed changes

pkg/controller/daemonset/daemonset_controller.go Outdated Show resolved Hide resolved

predownload image without checking updatedReadyPodsCount

cf1c5c6

Signed-off-by: Abner <abner199709@gmail.com>

ABNER-1 requested review from furykerry and removed request for FillZpp and Fei-Guo September 8, 2022 15:22

kruise-bot assigned furykerry Sep 13, 2022

kruise-bot added the lgtm label Sep 13, 2022

furykerry reviewed Sep 20, 2022

View reviewed changes

kruise-bot added the approved label Sep 28, 2022

FillZpp approved these changes Sep 28, 2022

View reviewed changes

FillZpp merged commit 9270d04 into openkruise:master Sep 28, 2022

ppbits pushed a commit to ppbits/kruise that referenced this pull request Apr 4, 2024

Support predownload image in ads (openkruise#1057)

971babd

* support predownload image for ads

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support predownload image in ads #1057

Support predownload image in ads #1057

ABNER-1 commented Aug 14, 2022

sonatype-lift bot Aug 14, 2022

ABNER-1 Aug 15, 2022

sonatype-lift bot Aug 15, 2022

sonatype-lift bot Aug 14, 2022

FillZpp commented Aug 16, 2022

ABNER-1 commented Aug 21, 2022

ABNER-1 commented Aug 24, 2022 •

edited

Loading

codecov-commenter commented Sep 8, 2022

furykerry commented Sep 13, 2022

furykerry Sep 20, 2022

FillZpp commented Sep 28, 2022

kruise-bot commented Sep 28, 2022

Support predownload image in ads #1057

Support predownload image in ads #1057

Conversation

ABNER-1 commented Aug 14, 2022

Ⅰ. Describe what this PR does

Ⅱ. Does this pull request fix one issue?

Ⅲ. Describe how to verify it

Ⅳ. Special notes for reviews

sonatype-lift bot Aug 14, 2022

Choose a reason for hiding this comment

ABNER-1 Aug 15, 2022

Choose a reason for hiding this comment

sonatype-lift bot Aug 15, 2022

Choose a reason for hiding this comment

sonatype-lift bot Aug 14, 2022

Choose a reason for hiding this comment

FillZpp commented Aug 16, 2022

ABNER-1 commented Aug 21, 2022

ABNER-1 commented Aug 24, 2022 • edited Loading

codecov-commenter commented Sep 8, 2022

Codecov Report

furykerry commented Sep 13, 2022

furykerry Sep 20, 2022

Choose a reason for hiding this comment

FillZpp commented Sep 28, 2022

kruise-bot commented Sep 28, 2022

ABNER-1 commented Aug 24, 2022 •

edited

Loading