Add proposal for PodMarker #704

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

Open

veophi wants to merge 2 commits into openkruise:master from veophi:podmarker-proposal

Member

veophi commented Aug 12, 2021

Signed-off-by: veophi [email protected]

Ⅰ. Describe what this PR does

Add proposal for PodMarker

Ⅱ. Does this pull request fix one issue?

fixes #640

kruise-bot requested review from Fei-Guo and FillZpp

August 12, 2021 08:03

kruise-bot added the size/L label

veophi force-pushed the podmarker-proposal branch from 54d25ce to 093378c Compare

August 12, 2021 08:05

codecov-commenter commented Aug 12, 2021 •

edited

Loading

Codecov Report

Merging #704 (f33c6dd) into master (34c1e98) will decrease coverage by 0.02%.
The diff coverage is n/a.

❗ Current head f33c6dd differs from pull request most recent head c8ac4cc. Consider uploading reports for the commit c8ac4cc to get more accurate results

@@            Coverage Diff             @@
##           master     #704      +/-   ##
==========================================
- Coverage   49.69%   49.67%   -0.03%     
==========================================
  Files         107      107              
  Lines        9773     9773              
==========================================
- Hits         4857     4855       -2     
- Misses       4202     4203       +1     
- Partials      714      715       +1

Flag	Coverage Δ
unittests	`49.67% <ø> (-0.03%)`	⬇️

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

Impacted Files	Coverage Δ
pkg/controller/uniteddeployment/revision.go	`66.15% <0.00%> (-1.54%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 34c1e98...c8ac4cc. Read the comment docs.

veophi force-pushed the podmarker-proposal branch 3 times, most recently from 3e20894 to f8e0c02 Compare

August 13, 2021 02:39

veophi force-pushed the podmarker-proposal branch 6 times, most recently from 4d08bc5 to 2c63e3a Compare

August 25, 2021 08:12

furykerry requested changes

View reviewed changes

docs/proposals/20210811-podmarker.md Outdated

    
              ****

              ### API Fantasy

              ```go

Member

furykerry Aug 25, 2021

api definition is not required in the proposals, and i suggest do not include them in the proposals to make the proposals more concise

docs/proposals/20210811-podmarker.md Outdated

    
                name: marker

              spec:

                strategy:

                  replica: 10

Member

furykerry Aug 25, 2021

replica -> replicas

docs/proposals/20210811-podmarker.md Outdated

    
                - `len(satisfiedPods) < Spec.Strategy.Replicas` if `Spec.Strategy.ConflictPolicy` is `"Overwrite"`.

                - `len(satisfiedPods) - len(conflictPods) < Spec.Strategy.Replicas` if `Spec.Strategy.ConflictPolicy` is `"Ignore"`.

              - The number(or percentage) of marked Pods `>` `Replicas` iff the number of already marked Pods `>` `Replicas`.

              - **Users can remove the marks marked by PodMarker by setting `.Strategy.Replicas = 0`.**

Member

furykerry Aug 25, 2021

what will happen if one reduce the replicas, will the annotations/labels of marked pods be reset or cleared ?

docs/proposals/20210811-podmarker.md Outdated

    
                  replica: 10

                  confilctPolicy: Ignore

                  sequencePolicy: NotReadyFirst

                  preferencesInFrontOfSequencePolicy: true

Member

furykerry Aug 25, 2021

is it possible to add ready term in preferences sections, and use an explicit weight to control the priority of each sorting terms?

veophi force-pushed the podmarker-proposal branch 3 times, most recently from 43f9efd to df674b2 Compare

August 25, 2021 08:42

furykerry requested changes

View reviewed changes

docs/proposals/20210811-podmarker.md Outdated

    
                  nodeSelector:

                    matchLabels:

                      arch: x86

                  probeRequirements:

Member

furykerry Aug 25, 2021

probeRequirements -> podProbe

Member Author

veophi Aug 25, 2021

done

docs/proposals/20210811-podmarker.md Outdated

    
              **Implement in Kruise-Daemon**

              We are going to implement the controller logic in `kruise-daemon` due to the following problems:

Member

furykerry Aug 25, 2021

Except probing, i don't think it is necessary to depend on kruise-daemon, a controller in kruise-manager is enough

Member Author

veophi Aug 25, 2021

I agree

docs/proposals/20210811-podmarker.md Outdated

    
                strategy:

                  replicas: 10

                  confilctPolicy: Ignore

                requirements:

Member

furykerry Aug 25, 2021

consider make matching criteria naming more consistent
requirements -> matchRequirements
sortRuleForMatchedPods -> matchPreferences

Member Author

veophi Aug 25, 2021

done

veophi force-pushed the podmarker-proposal branch 4 times, most recently from 699c2b0 to ce4aa62 Compare

August 26, 2021 09:19

furykerry requested changes

View reviewed changes

docs/proposals/20210811-podmarker.md Outdated Show resolved Hide resolved

docs/proposals/20210811-podmarker.md Outdated Show resolved Hide resolved

veophi force-pushed the podmarker-proposal branch 3 times, most recently from ed83258 to 08680d4 Compare

August 27, 2021 05:40

veophi mentioned this pull request

[REQUEST Membership]: New membership for <veophi> openkruise/community#4

Closed

6 tasks

veophi force-pushed the podmarker-proposal branch 2 times, most recently from f33c6dd to de2cd3c Compare

September 7, 2021 06:46

veophi added 2 commits

September 7, 2021 14:47


          add proposal for PodMarker

36eefdc

Signed-off-by: veophi <[email protected]>


          update special cases, event watch and webhook description

c8ac4cc

Signed-off-by: veophi <[email protected]>

veophi force-pushed the podmarker-proposal branch from de2cd3c to c8ac4cc Compare

September 7, 2021 06:47

furykerry reviewed

View reviewed changes

Member

furykerry left a comment

/lgtm

kruise-bot assigned furykerry

kruise-bot added the lgtm label

stale bot commented Dec 6, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale bot added the wontfix label

Member

FillZpp commented Dec 6, 2021

/pinned

stale bot removed the wontfix label

FillZpp mentioned this pull request

[feature request] PROPOSAL for CloneSet "Downscale Pod Picker" API #902

Closed

stale bot commented May 24, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale bot added the wontfix label

FillZpp removed the wontfix label

stale bot commented Aug 31, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale bot added the wontfix label

stale bot closed this

zmberg reopened this

kruise-bot commented Jan 5, 2023

[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 furykerry by writing /assign @furykerry 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:

OWNERS

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

stale bot removed the wontfix label

Member

zmberg commented Jan 5, 2023

stale bot commented Apr 5, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale bot added the wontfix label

stale bot closed this

furykerry removed the wontfix label

zmberg reopened this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels