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

adding label to openshift-customer-monitoring #184

Merged
merged 1 commit into from Jan 14, 2020

Conversation

@RiRa12621
Copy link
Member

RiRa12621 commented Jan 13, 2020

To allow for network polices to be used from and to openshift-customer-monitoring, we have to have a label identifying that namespace.
I propose to be straight forward and use name

@RiRa12621 RiRa12621 requested a review from rogbas Jan 13, 2020
Copy link
Member

cblecker left a comment

This feels like it could get confusing.. having both .metadata.name and .metadata.labels[name], and it isn’t clear what the label is for. Would it be better to be more explicit?

@RiRa12621

This comment has been minimized.

Copy link
Member Author

RiRa12621 commented Jan 13, 2020

Sure, do you have anything in particular in mind?

@maorfr

This comment has been minimized.

Copy link
Member

maorfr commented Jan 13, 2020

fwiw, the following namespaces have the name label:

openshift-cluster-version
openshift-ingress
openshift-insights
openshift-kni-infra
openshift-machine-api
openshift-machine-config-operator
openshift-monitoring
openshift-multus
openshift-network-operator
openshift-openstack-infra
openshift-sdn

@rogbas

This comment has been minimized.

Copy link
Contributor

rogbas commented Jan 13, 2020

I agree it's confusing, but the name label is used by the networkPolicy label selector, and several other NSs have to use it too.

IMO, updating that across the board should be covered in a different card.

/lgtm

@rogbas
rogbas approved these changes Jan 13, 2020
@openshift-ci-robot

This comment has been minimized.

Copy link

openshift-ci-robot commented Jan 13, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mwoodson, RiRa12621, rogbas

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 [RiRa12621,mwoodson,rogbas]

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

@jewzaam

This comment has been minimized.

Copy link
Member

jewzaam commented Jan 13, 2020

/retest

@cblecker

This comment has been minimized.

Copy link
Member

cblecker commented Jan 13, 2020

I agree it's confusing, but the name label is used by the networkPolicy label selector, and several other NSs have to use it too.

Ah, that makes sense. Yes, I'm okay with this then too 👍

@RiRa12621

This comment has been minimized.

Copy link
Member Author

RiRa12621 commented Jan 14, 2020

great can someone /lgtm please ?

@RiRa12621

This comment has been minimized.

Copy link
Member Author

RiRa12621 commented Jan 14, 2020

/retest

@RiRa12621 RiRa12621 merged commit 28231d9 into master Jan 14, 2020
3 checks passed
3 checks passed
ci.ext.devshift.net PR build
Details
ci/prow/images Job succeeded.
Details
tide In merge pool.
Details
@RiRa12621 RiRa12621 deleted the label-customer-monitoring-namespace branch Jan 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.