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.

Sign up for GitHub

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

Jump to bottom

pub support custom workload #982

Merged
merged 1 commit into from
May 27, 2022
Merged

pub support custom workload #982

merged 1 commit into from
May 27, 2022

Conversation

zmberg
Copy link
Member

@zmberg zmberg commented May 24, 2022 •
edited
Loading

Signed-off-by: liheng.zms [email protected]

Ⅰ. Describe what this PR does

  1. pub support custom workload, for example argo-rollouts. The custom workload must contain scale subresources.
  2. add pub annotations[kruise.io/pub-feature-gate] to determine the feature of the protection for each pub object. the following indicates that delete protection is enabled and update protection is disabled
    annotations[kruise.io/pub-feature-gate]='PodUnavailableBudgetUpdateGate=false,PodUnavailableBudgetDeleteGate=true'

@kruise-bot kruise-bot added the size/L size/L: 100-499 label May 24, 2022
sonatype-lift[bot]
sonatype-lift bot reviewed May 24, 2022
View reviewed changes
pkg/util/controllerfinder/controller_finder.go Outdated
"k8s.io/client-go/discovery"
"k8s.io/client-go/dynamic"
clientset "k8s.io/client-go/kubernetes"
"k8s.io/client-go/scale"
Copy link

Choose a reason for hiding this comment

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

ST1019: package "k8s.io/client-go/scale" is being imported more than once

(at-me in a reply with help or ignore)

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

sonatype-lift[bot]
sonatype-lift bot reviewed May 24, 2022
View reviewed changes
pkg/util/controllerfinder/pods_finder.go Outdated
@@ -18,7 +18,6 @@ package controllerfinder

import (
"context"
Copy link

Choose a reason for hiding this comment

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

goimports: File is not goimports-ed

(at-me in a reply with help or ignore)

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

sonatype-lift[bot]
sonatype-lift bot reviewed May 24, 2022
View reviewed changes
main.go Outdated
@@ -18,6 +18,7 @@ package main

import (
"flag"
"github.com/openkruise/kruise/pkg/util/controllerfinder"
Copy link

Choose a reason for hiding this comment

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

goimports: File is not goimports-ed

(at-me in a reply with help or ignore)

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

@zmberg zmberg force-pushed the pub_scale_resources branch 2 times, most recently from 11ea421 to cf71458 Compare May 25, 2022 09:26
sonatype-lift[bot]
sonatype-lift bot reviewed May 25, 2022
View reviewed changes
pkg/webhook/pod/validating/pod_unavailable_budget.go
@@ -18,6 +18,9 @@ package validating

import (
"context"
Copy link

Choose a reason for hiding this comment

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

goimports: File is not goimports-ed

(at-me in a reply with help or ignore)

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

@zmberg zmberg force-pushed the pub_scale_resources branch from cf71458 to 856255b Compare May 25, 2022 12:52
@kruise-bot kruise-bot added size/XL size/XL: 500-999 and removed size/L size/L: 100-499 labels May 25, 2022
@zmberg zmberg force-pushed the pub_scale_resources branch 2 times, most recently from a288bfa to b1f4f48 Compare May 26, 2022 02:35
@zmberg zmberg force-pushed the pub_scale_resources branch from b1f4f48 to 53db277 Compare May 26, 2022 08:52
@kruise-bot kruise-bot added size/L size/L: 100-499 and removed size/XL size/XL: 500-999 labels May 26, 2022
@zmberg zmberg force-pushed the pub_scale_resources branch from 53db277 to 27d112c Compare May 26, 2022 09:49
@FillZpp
Copy link
Member

FillZpp commented May 26, 2022

/lgtm

furykerry
furykerry reviewed May 26, 2022
View reviewed changes
pkg/util/controllerfinder/pods_finder.go Outdated
return nil, 0, nil
}
workloadReplicas = obj.Scale
workloadUIDs = append(workloadUIDs, obj.UID)
// Deployment and others crds
Copy link
Member

Choose a reason for hiding this comment

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

there is special logic to extract replicaset for deployment, is it better to separate deployment handling from general scale resource handling?

furykerry
furykerry reviewed May 26, 2022
View reviewed changes
apis/policy/v1alpha1/podunavailablebudget_types.go Outdated
const (
// PubProtectOperationAnnotation indicates the pub protected Operation[DELETE,UPDATE]
// the following indicates the pub only protect DELETE Operation
// annotations[kruise.io/pub-protect-operations]=DELETE
Copy link
Member

Choose a reason for hiding this comment

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

please demonstrate how to protect multiple operation explicitly

kruise.io/pub-protect-operations: DELETE,UPDATE

@zmberg zmberg force-pushed the pub_scale_resources branch from 27d112c to 95844ce Compare May 26, 2022 11:40
@kruise-bot kruise-bot removed the lgtm label May 26, 2022
furykerry
furykerry reviewed May 26, 2022
View reviewed changes
test/e2e/apps/daemonset.go Outdated
@@ -55,7 +55,7 @@ var _ = SIGDescribe("DaemonSet", func() {
*/
framework.ConformanceIt("should run and stop simple daemon", func() {
label := map[string]string{framework.DaemonSetNameLabel: dsName}

fmt.Println("start")
Copy link
Member

Choose a reason for hiding this comment

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

remote temp code

@zmberg zmberg force-pushed the pub_scale_resources branch from 95844ce to 7697962 Compare May 26, 2022 12:08
furykerry
furykerry reviewed May 26, 2022
View reviewed changes
Copy link
Member

@furykerry furykerry left a comment

Choose a reason for hiding this comment

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

/lgtm

@FillZpp
Copy link
Member

FillZpp commented May 27, 2022

/approve

@kruise-bot
Copy link

[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:

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 7d093af into openkruise:master May 27, 2022
@zmberg zmberg deleted the pub_scale_resources branch May 27, 2022 02:14
@FillZpp FillZpp mentioned this pull request Aug 16, 2022
Closed
diannaowa pushed a commit to diannaowa/kruise that referenced this pull request Sep 14, 2022
@zmberg @diannaowa
pub support custom workload (openkruise#982)
36f2ac9 
Signed-off-by: liheng.zms <[email protected]>
Signed-off-by: Liu Zhenwei <[email protected]>
ppbits pushed a commit to ppbits/kruise that referenced this pull request Apr 4, 2024
@zmberg
pub support custom workload (openkruise#982)
a6f6547
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sonatype-lift sonatype-lift[bot] sonatype-lift[bot] left review comments

@FillZpp FillZpp FillZpp left review comments

@furykerry furykerry furykerry left review comments

@shiyan2016 shiyan2016 Awaiting requested review from shiyan2016

Assignees

@furykerry furykerry

@FillZpp FillZpp

Labels
approved lgtm size/L size/L: 100-499
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@zmberg @FillZpp @kruise-bot @furykerry