Guidelines: set matchLabels as being mandatory #7692

desaintmartin · 2018-09-12T12:20:04Z

Also move "multiple components" hint and separate the 3 different topics.

See https://github.com/helm/charts/issues/3011 and #7680

desaintmartin · 2018-09-12T12:21:19Z

cc @scottrigby

REVIEW_GUIDELINES.md

desaintmartin · 2018-09-14T21:49:40Z

I've pushed one more commit introducing an upgrade path for selector. I am not sure it is the right place to write it, so feel free to discuss.

REVIEW_GUIDELINES.md

unguiculus · 2018-09-16T12:09:38Z

Could you also update the persistence section in the guidelines to include matchLabels?

unguiculus · 2018-09-17T07:32:44Z

Not sure about PVC. Looks like labels work differently there. I suggested to add matchLabels for the PVC in #5987 which causes the PVC to stay pending forever.

desaintmartin · 2018-09-17T14:58:51Z

I think matchLabels for PVC are related to their PV counterpart, which (hopefully) is out of scope. I'd be glad to see confirmation.

I've followed your change requests and created a few discussions.

REVIEW_GUIDELINES.md

desaintmartin · 2018-09-17T15:05:07Z

/do-not-merge
Edit: it seems I can't prevent it, can someone add it until we finish the discussion to prevent merge by mistake?

mattfarina · 2018-10-16T16:52:58Z

@desaintmartin @unguiculus @cpanato Can we remove the hold. This LGTM.

cpanato · 2018-10-16T16:54:33Z

/hold cancel

desaintmartin · 2018-10-19T08:24:42Z

Should we merge? @unguiculus any remark about spec.volumeClaimTemplates.metadata.labels ?

mattfarina · 2018-10-19T13:38:28Z

/ok-to-test

Signed-off-by: Cédric de Saint Martin <cdesaintmartin@wiremind.fr>

…peat component part. Signed-off-by: Cédric de Saint Martin <cdesaintmartin@wiremind.fr>

Signed-off-by: Cédric de Saint Martin <cdesaintmartin@wiremind.fr>

Signed-off-by: Reinhard Nägele <unguiculus@gmail.com>

unguiculus · 2018-10-24T18:29:53Z

I think we can merge this.

unguiculus · 2018-10-24T18:30:02Z

/lgtm

k8s-ci-robot · 2018-10-24T18:30:10Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: desaintmartin, unguiculus

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~~ [unguiculus]

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

* Guidelines: set matchLabels as being mandatory. Signed-off-by: Cédric de Saint Martin <cdesaintmartin@wiremind.fr> * fixup! Guidelines: set matchLabels as being mandatory. Signed-off-by: Cédric de Saint Martin <cdesaintmartin@wiremind.fr> * fixup! fixup! Guidelines: set matchLabels as being mandatory. Signed-off-by: Cédric de Saint Martin <cdesaintmartin@wiremind.fr> * Update guidelines: mention DaemonSets as well. Signed-off-by: Cédric de Saint Martin <cdesaintmartin@wiremind.fr> * Review guidelines: be more precise + specify upgrade Signed-off-by: Cédric de Saint Martin <cdesaintmartin@wiremind.fr> * Review guidelines: fix typos, add persistence paragraph and do not repeat component part. Signed-off-by: Cédric de Saint Martin <cdesaintmartin@wiremind.fr> * Review guidelines: add PVC paragraph. Signed-off-by: Cédric de Saint Martin <cdesaintmartin@wiremind.fr> * Fix spelling/typos Signed-off-by: Reinhard Nägele <unguiculus@gmail.com> * Fix incorrect typo fix Signed-off-by: Reinhard Nägele <unguiculus@gmail.com>

* Guidelines: set matchLabels as being mandatory. Signed-off-by: Cédric de Saint Martin <cdesaintmartin@wiremind.fr> * fixup! Guidelines: set matchLabels as being mandatory. Signed-off-by: Cédric de Saint Martin <cdesaintmartin@wiremind.fr> * fixup! fixup! Guidelines: set matchLabels as being mandatory. Signed-off-by: Cédric de Saint Martin <cdesaintmartin@wiremind.fr> * Update guidelines: mention DaemonSets as well. Signed-off-by: Cédric de Saint Martin <cdesaintmartin@wiremind.fr> * Review guidelines: be more precise + specify upgrade Signed-off-by: Cédric de Saint Martin <cdesaintmartin@wiremind.fr> * Review guidelines: fix typos, add persistence paragraph and do not repeat component part. Signed-off-by: Cédric de Saint Martin <cdesaintmartin@wiremind.fr> * Review guidelines: add PVC paragraph. Signed-off-by: Cédric de Saint Martin <cdesaintmartin@wiremind.fr> * Fix spelling/typos Signed-off-by: Reinhard Nägele <unguiculus@gmail.com> * Fix incorrect typo fix Signed-off-by: Reinhard Nägele <unguiculus@gmail.com> Signed-off-by: Jakob Niggel <info@jakobniggel.de>

* Guidelines: set matchLabels as being mandatory. Signed-off-by: Cédric de Saint Martin <cdesaintmartin@wiremind.fr> * fixup! Guidelines: set matchLabels as being mandatory. Signed-off-by: Cédric de Saint Martin <cdesaintmartin@wiremind.fr> * fixup! fixup! Guidelines: set matchLabels as being mandatory. Signed-off-by: Cédric de Saint Martin <cdesaintmartin@wiremind.fr> * Update guidelines: mention DaemonSets as well. Signed-off-by: Cédric de Saint Martin <cdesaintmartin@wiremind.fr> * Review guidelines: be more precise + specify upgrade Signed-off-by: Cédric de Saint Martin <cdesaintmartin@wiremind.fr> * Review guidelines: fix typos, add persistence paragraph and do not repeat component part. Signed-off-by: Cédric de Saint Martin <cdesaintmartin@wiremind.fr> * Review guidelines: add PVC paragraph. Signed-off-by: Cédric de Saint Martin <cdesaintmartin@wiremind.fr> * Fix spelling/typos Signed-off-by: Reinhard Nägele <unguiculus@gmail.com> * Fix incorrect typo fix Signed-off-by: Reinhard Nägele <unguiculus@gmail.com>

ey-bot added the Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). label Sep 12, 2018

k8s-ci-robot requested review from davidkarlsen and scottrigby September 12, 2018 12:20

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Sep 12, 2018

scottrigby reviewed Sep 12, 2018

View reviewed changes

REVIEW_GUIDELINES.md Outdated Show resolved Hide resolved

scottrigby mentioned this pull request Sep 12, 2018

Putting chart version in spec.selector.matchLabels in apps/v1beta2 and later prevents ANY future upgrade of existing Release #7680

Closed

desaintmartin mentioned this pull request Sep 12, 2018

start on docs update for labels #7696

Closed

ey-bot added the Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). label Sep 13, 2018

desaintmartin commented Sep 13, 2018

View reviewed changes

REVIEW_GUIDELINES.md Outdated Show resolved Hide resolved

unguiculus suggested changes Sep 14, 2018

View reviewed changes

REVIEW_GUIDELINES.md Outdated Show resolved Hide resolved

unguiculus assigned unguiculus, mattfarina and scottrigby Sep 14, 2018

ey-bot added Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). and removed Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). labels Sep 14, 2018

scottrigby mentioned this pull request Sep 14, 2018

Putting chart version in metadata.labels in a Deployment v1beta1 without selector prevents upgrade #7726

Closed

ey-bot added Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). and removed Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). labels Sep 14, 2018

unguiculus suggested changes Sep 16, 2018

View reviewed changes

ey-bot removed the Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). label Sep 17, 2018

desaintmartin commented Sep 17, 2018

View reviewed changes

REVIEW_GUIDELINES.md Outdated Show resolved Hide resolved

desaintmartin commented Sep 17, 2018

View reviewed changes

REVIEW_GUIDELINES.md Show resolved Hide resolved

desaintmartin commented Sep 17, 2018

View reviewed changes

REVIEW_GUIDELINES.md Outdated Show resolved Hide resolved

ey-bot added the Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). label Sep 17, 2018

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 16, 2018

k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Oct 19, 2018

desaintmartin added 7 commits October 24, 2018 19:26

Guidelines: set matchLabels as being mandatory.

7eee797

Signed-off-by: Cédric de Saint Martin <cdesaintmartin@wiremind.fr>

fixup! Guidelines: set matchLabels as being mandatory.

8c934de

Signed-off-by: Cédric de Saint Martin <cdesaintmartin@wiremind.fr>

fixup! fixup! Guidelines: set matchLabels as being mandatory.

08bb0ff

Signed-off-by: Cédric de Saint Martin <cdesaintmartin@wiremind.fr>

Update guidelines: mention DaemonSets as well.

699bc6e

Signed-off-by: Cédric de Saint Martin <cdesaintmartin@wiremind.fr>

Review guidelines: be more precise + specify upgrade

18e7eb5

Signed-off-by: Cédric de Saint Martin <cdesaintmartin@wiremind.fr>

Review guidelines: fix typos, add persistence paragraph and do not re…

1b7367c

…peat component part. Signed-off-by: Cédric de Saint Martin <cdesaintmartin@wiremind.fr>

Review guidelines: add PVC paragraph.

2f4a9c7

Signed-off-by: Cédric de Saint Martin <cdesaintmartin@wiremind.fr>

helm-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Oct 24, 2018

Fix spelling/typos

1a00d50

Signed-off-by: Reinhard Nägele <unguiculus@gmail.com>

helm-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 24, 2018

Fix incorrect typo fix

169ea0c

Signed-off-by: Reinhard Nägele <unguiculus@gmail.com>

helm-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 24, 2018

unguiculus approved these changes Oct 24, 2018

View reviewed changes

k8s-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 24, 2018

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 24, 2018

k8s-ci-robot merged commit 234f0eb into helm:master Oct 24, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Guidelines: set matchLabels as being mandatory #7692

Guidelines: set matchLabels as being mandatory #7692

desaintmartin commented Sep 12, 2018

desaintmartin commented Sep 12, 2018

desaintmartin commented Sep 14, 2018

unguiculus commented Sep 16, 2018

unguiculus commented Sep 17, 2018

desaintmartin commented Sep 17, 2018 •

edited

Loading

desaintmartin commented Sep 17, 2018 •

edited

Loading

mattfarina commented Oct 16, 2018

cpanato commented Oct 16, 2018

desaintmartin commented Oct 19, 2018

mattfarina commented Oct 19, 2018

unguiculus commented Oct 24, 2018

unguiculus commented Oct 24, 2018

k8s-ci-robot commented Oct 24, 2018

Guidelines: set matchLabels as being mandatory #7692

Guidelines: set matchLabels as being mandatory #7692

Conversation

desaintmartin commented Sep 12, 2018

desaintmartin commented Sep 12, 2018

desaintmartin commented Sep 14, 2018

unguiculus commented Sep 16, 2018

unguiculus commented Sep 17, 2018

desaintmartin commented Sep 17, 2018 • edited Loading

desaintmartin commented Sep 17, 2018 • edited Loading

mattfarina commented Oct 16, 2018

cpanato commented Oct 16, 2018

desaintmartin commented Oct 19, 2018

mattfarina commented Oct 19, 2018

unguiculus commented Oct 24, 2018

unguiculus commented Oct 24, 2018

k8s-ci-robot commented Oct 24, 2018

desaintmartin commented Sep 17, 2018 •

edited

Loading

desaintmartin commented Sep 17, 2018 •

edited

Loading