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

Define labels for route/ingress #449

Merged
merged 13 commits into from
Sep 25, 2020
Merged

Define labels for route/ingress #449

merged 13 commits into from
Sep 25, 2020

Conversation

tolusha
Copy link
Contributor

@tolusha tolusha commented Sep 17, 2020

Signed-off-by: Anatolii Bazko abazko@redhat.com

What does this PR do

Possibility to define label(s) for route to work with router sharding

Reference issue

https://issues.redhat.com/browse/CRW-1034

Copy link
Contributor

@davidfestal davidfestal left a comment

Choose a reason for hiding this comment

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

I'm not very convinced about the use of new CheClusterSpecDevfileRegistry and CheClusterSpecPluginRegistry sections.

These new sections would in fact contain only the label-related field, while most other registry-related settings would still be in the "Server" section.
Of course adding 2 new sections would completely make sense if we were ready to refactor the schema completely, and bring all the registry-related settings in the dedicated registry section. But I assume we would not start such a refactoring now since it which would break backward compatibility of the CRD schema.

So I'd rather follow the existing pattern, and use names like devfileRegistryLabels in the server section since we already had devfileRegistryImage, devfileRegistryPullPolicy, ...

Signed-off-by: Anatolii Bazko <abazko@redhat.com>
Signed-off-by: Anatolii Bazko <abazko@redhat.com>
Signed-off-by: Anatolii Bazko <abazko@redhat.com>
Signed-off-by: Anatolii Bazko <abazko@redhat.com>
Signed-off-by: Anatolii Bazko <abazko@redhat.com>
Signed-off-by: Anatolii Bazko <abazko@redhat.com>
Signed-off-by: Anatolii Bazko <abazko@redhat.com>
Signed-off-by: Anatolii Bazko <abazko@redhat.com>
Signed-off-by: Anatolii Bazko <abazko@redhat.com>
Signed-off-by: Anatolii Bazko <abazko@redhat.com>
Signed-off-by: Anatolii Bazko <abazko@redhat.com>
Signed-off-by: Anatolii Bazko <abazko@redhat.com>
Signed-off-by: Anatolii Bazko <abazko@redhat.com>
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: AndrienkoAleksandr, davidfestal, tolusha
To complete the pull request process, please assign
You can assign the PR to them by writing /assign in a comment when ready.

The full list of commands accepted by this bot can be found 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

@tolusha tolusha merged commit 37e6a9d into master Sep 25, 2020
@tolusha tolusha deleted the additionallabels branch September 25, 2020 11:17
@tolusha tolusha restored the additionallabels branch September 25, 2020 11:17
@tolusha tolusha deleted the additionallabels branch September 25, 2020 11:17
@che-bot che-bot added this to the 7.20 milestone Sep 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants