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

support bluegreen release: support workload of deployment and cloneSet #238

Merged
merged 6 commits into from
Dec 24, 2024

Conversation

myname4423
Copy link
Contributor

Ⅰ. Describe what this PR does

  1. [mainly] add control logic for both deployment and cloneSet under controller/batchrelease, to support bluegreen style release
  2. add e2e for both deployment and cloneSet for bluegreen

Ⅱ. Does this pull request fix one issue?

Ⅲ. Special notes for reviews

util.InitOriginalSetting(&setting, rc.object)
klog.InfoS("Initialize deployment", "deployment", klog.KObj(rc.object), "setting", util.DumpJSON(&setting))

d := rc.object.DeepCopy()
Copy link
Member

Choose a reason for hiding this comment

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

the obj params of patch field is used to pass the object meta and to store the patched obj, since d is not used anywhere, it is not necessary to deepcopy the object. simply initiate a empty deployment with the namespace/name of obj using GetEmptyObjectWithKey

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link

codecov bot commented Nov 19, 2024

Codecov Report

Attention: Patch coverage is 38.69801% with 339 lines in your changes missing coverage. Please review.

Project coverage is 45.00%. Comparing base (07c1731) to head (ab52d6c).
Report is 15 commits behind head on master.

Files with missing lines Patch % Lines
pkg/util/patch/patch_utils.go 0.00% 93 Missing ⚠️
pkg/controller/batchrelease/control/util.go 0.00% 75 Missing ⚠️
...lease/control/bluegreenstyle/deployment/control.go 60.66% 45 Missing and 14 partials ⚠️
...release/control/bluegreenstyle/cloneset/control.go 57.69% 31 Missing and 13 partials ⚠️
...g/controller/batchrelease/batchrelease_executor.go 13.63% 19 Missing ⚠️
...ler/batchrelease/control/bluegreenstyle/hpa/hpa.go 64.81% 10 Missing and 9 partials ⚠️
pkg/util/condition.go 0.00% 13 Missing ⚠️
...bhook/workload/mutating/workload_update_handler.go 68.57% 8 Missing and 3 partials ⚠️
pkg/controller/rollout/rollout_bluegreen.go 0.00% 1 Missing and 1 partial ⚠️
pkg/controller/rollout/rollout_canary.go 0.00% 1 Missing and 1 partial ⚠️
... and 1 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #238      +/-   ##
==========================================
+ Coverage   43.63%   45.00%   +1.36%     
==========================================
  Files          52       60       +8     
  Lines        5681     6931    +1250     
==========================================
+ Hits         2479     3119     +640     
- Misses       2778     3279     +501     
- Partials      424      533     +109     
Flag Coverage Δ
unittests 45.00% <38.69%> (+1.36%) ⬆️

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.


🚨 Try these New Features:

pkg/controller/batchrelease/control/util.go Outdated Show resolved Hide resolved
pkg/webhook/util/revision/revision.go Outdated Show resolved Hide resolved
pkg/webhook/util/revision/revision.go Outdated Show resolved Hide resolved
pkg/webhook/workload/mutating/workload_update_handler.go Outdated Show resolved Hide resolved
pkg/webhook/workload/mutating/workload_update_handler.go Outdated Show resolved Hide resolved
@myname4423 myname4423 force-pushed the bluegreen branch 2 times, most recently from b8bd5b5 to 6fed8e1 Compare December 4, 2024 06:03
pkg/util/errors/types.go Outdated Show resolved Hide resolved
@furykerry
Copy link
Member

/lgtm

@@ -148,6 +148,7 @@ type BatchReleaseReconciler struct {
// +kubebuilder:rbac:groups=apps.kruise.io,resources=statefulsets/status,verbs=get;update;patch
// +kubebuilder:rbac:groups=apps.kruise.io,resources=daemonsets,verbs=get;list;watch;update;patch
// +kubebuilder:rbac:groups=apps.kruise.io,resources=daemonsets/status,verbs=get;update;patch
// +kubebuilder:rbac:groups=autoscaling,resources=horizontalpodautoscalers,verbs=get;list;watch;create;update;patch;delete
Copy link
Member

Choose a reason for hiding this comment

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

verbs=get;list;watch;update;patch

}

func (rc *realController) Initialize(release *v1beta1.BatchRelease) error {
klog.Info("Initialize cloneset controller")
Copy link
Member

Choose a reason for hiding this comment

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

remove the log

desired, _ := intstr.GetScaledValueFromIntOrPercent(&ctx.DesiredSurge, int(ctx.Replicas), true)
current, _ := intstr.GetScaledValueFromIntOrPercent(&ctx.CurrentSurge, int(ctx.Replicas), true)
if current >= desired {
klog.Infof("No need to upgrade batch for cloneset %v: because current %d >= desired %d", klog.KObj(rc.object), current, desired)
Copy link
Member

Choose a reason for hiding this comment

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

infos

return errors.NewRetryError(fmt.Errorf("cloneset %v finalize not done, readyReplicas %d != updatedReadyReplicas %d, current policy %s",
klog.KObj(rc.object), rc.object.Status.ReadyReplicas, rc.object.Status.UpdatedReadyReplicas, release.Spec.ReleasePlan.FinalizingPolicy))
}
klog.InfoS("Finalize: cloneset bluegreen release: all pods updated and ready")
Copy link
Member

Choose a reason for hiding this comment

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

add klog.KObj(rc.object).

@zmberg zmberg merged commit efbd8ba into openkruise:master Dec 24, 2024
23 of 24 checks passed
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.

4 participants