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

feat: add support for custom service labels #1952

Merged

Conversation

Cian911
Copy link
Contributor

@Cian911 Cian911 commented Apr 6, 2024

There is no way currently to add custom labels to services created by the spark-operator. This can be very useful for anyone who uses opa-gatekeeper for policy controls in their kubernetes clusters.

See related issues of users asking for this: #1342

Copy link
Contributor

@yuchaoran2011 yuchaoran2011 left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution. Indentation has issues. Please make sure to run go fmt to reformat the code. serviceLabels sounds generic. Can we make it clear by renaming it to something like sparkUiSvcLabels?

Signed-off-by: Cian Gallagher <[email protected]>
@Cian911
Copy link
Contributor Author

Cian911 commented Apr 7, 2024

Thanks for the contribution. Indentation has issues. Please make sure to run go fmt to reformat the code. serviceLabels sounds generic. Can we make it clear by renaming it to something like sparkUiSvcLabels?

@yuchaoran2011 Thanks for taking a look, and the quick response. I have pushed a fix for the formatting 👍

However, with regards to renaming serviceLabels, I'm not sure I agree. The change I made, I feel, is adhering to the naming already in place for other attributes on the SparkUIConfiguration struct: see here.

I can make this change if insisted, but I personally don't see the benefit in the following configuration (as this suggestion might entail):

sparkUIOptions:
    sparkUiSvcLabels:
      test-label/v1: 'true'

vs

sparkUIOptions:
    serviceLabels:
      test-label/v1: 'true'

Copy link
Contributor

@yuchaoran2011 yuchaoran2011 left a comment

Choose a reason for hiding this comment

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

You are right. I missed that the newly added serviceLabels is under UI configurations. That's good then. One last change: another PR got merged before yours that took the 1.1.29 version number, now the chart number needs to be bump up to 1.1.30

@Cian911
Copy link
Contributor Author

Cian911 commented Apr 7, 2024

@yuchaoran2011 Done :)

Copy link
Contributor

@yuchaoran2011 yuchaoran2011 left a comment

Choose a reason for hiding this comment

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

/lgtm

Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Cian911, yuchaoran2011

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

@google-oss-prow google-oss-prow bot merged commit 8a31a46 into kubeflow:master Apr 7, 2024
2 of 3 checks passed
AndrewChubatiuk pushed a commit to AndrewChubatiuk/spark-on-k8s-operator that referenced this pull request Apr 7, 2024
* feat: add support for custom service labels

Signed-off-by: Cian Gallagher <[email protected]>

* chore: correctly format files

Signed-off-by: Cian Gallagher <[email protected]>

* chore: bump chart version to 1.1.30

Signed-off-by: Cian Gallagher <[email protected]>

---------

Signed-off-by: Cian Gallagher <[email protected]>
Signed-off-by: Andrew Chubatiuk <[email protected]>
@Cian911 Cian911 deleted the cian911/add-custom-svc-labels branch April 7, 2024 21:33
google-oss-prow bot pushed a commit that referenced this pull request Apr 8, 2024
* cleanup after upgrade

Signed-off-by: Andrew Chubatiuk <[email protected]>

* pr comments

Signed-off-by: Andrew Chubatiuk <[email protected]>

* feat: add support for custom service labels (#1952)

* feat: add support for custom service labels

Signed-off-by: Cian Gallagher <[email protected]>

* chore: correctly format files

Signed-off-by: Cian Gallagher <[email protected]>

* chore: bump chart version to 1.1.30

Signed-off-by: Cian Gallagher <[email protected]>

---------

Signed-off-by: Cian Gallagher <[email protected]>
Signed-off-by: Andrew Chubatiuk <[email protected]>

---------

Signed-off-by: Andrew Chubatiuk <[email protected]>
Signed-off-by: Cian Gallagher <[email protected]>
Co-authored-by: Cian Gallagher <[email protected]>
peter-mcclonski pushed a commit to TechnologyBrewery/spark-on-k8s-operator that referenced this pull request Apr 16, 2024
* feat: add support for custom service labels

Signed-off-by: Cian Gallagher <[email protected]>

* chore: correctly format files

Signed-off-by: Cian Gallagher <[email protected]>

* chore: bump chart version to 1.1.30

Signed-off-by: Cian Gallagher <[email protected]>

---------

Signed-off-by: Cian Gallagher <[email protected]>
Signed-off-by: Peter McClonski <[email protected]>
peter-mcclonski pushed a commit to TechnologyBrewery/spark-on-k8s-operator that referenced this pull request Apr 16, 2024
* cleanup after upgrade

Signed-off-by: Andrew Chubatiuk <[email protected]>

* pr comments

Signed-off-by: Andrew Chubatiuk <[email protected]>

* feat: add support for custom service labels (kubeflow#1952)

* feat: add support for custom service labels

Signed-off-by: Cian Gallagher <[email protected]>

* chore: correctly format files

Signed-off-by: Cian Gallagher <[email protected]>

* chore: bump chart version to 1.1.30

Signed-off-by: Cian Gallagher <[email protected]>

---------

Signed-off-by: Cian Gallagher <[email protected]>
Signed-off-by: Andrew Chubatiuk <[email protected]>

---------

Signed-off-by: Andrew Chubatiuk <[email protected]>
Signed-off-by: Cian Gallagher <[email protected]>
Co-authored-by: Cian Gallagher <[email protected]>
Signed-off-by: Peter McClonski <[email protected]>
sigmarkarl pushed a commit to spotinst/spark-on-k8s-operator that referenced this pull request Aug 7, 2024
* feat: add support for custom service labels

Signed-off-by: Cian Gallagher <[email protected]>

* chore: correctly format files

Signed-off-by: Cian Gallagher <[email protected]>

* chore: bump chart version to 1.1.30

Signed-off-by: Cian Gallagher <[email protected]>

---------

Signed-off-by: Cian Gallagher <[email protected]>
sigmarkarl pushed a commit to spotinst/spark-on-k8s-operator that referenced this pull request Aug 7, 2024
* cleanup after upgrade

Signed-off-by: Andrew Chubatiuk <[email protected]>

* pr comments

Signed-off-by: Andrew Chubatiuk <[email protected]>

* feat: add support for custom service labels (kubeflow#1952)

* feat: add support for custom service labels

Signed-off-by: Cian Gallagher <[email protected]>

* chore: correctly format files

Signed-off-by: Cian Gallagher <[email protected]>

* chore: bump chart version to 1.1.30

Signed-off-by: Cian Gallagher <[email protected]>

---------

Signed-off-by: Cian Gallagher <[email protected]>
Signed-off-by: Andrew Chubatiuk <[email protected]>

---------

Signed-off-by: Andrew Chubatiuk <[email protected]>
Signed-off-by: Cian Gallagher <[email protected]>
Co-authored-by: Cian Gallagher <[email protected]>
jbhalodia-slack pushed a commit to jbhalodia-slack/spark-operator that referenced this pull request Oct 4, 2024
* feat: add support for custom service labels

Signed-off-by: Cian Gallagher <[email protected]>

* chore: correctly format files

Signed-off-by: Cian Gallagher <[email protected]>

* chore: bump chart version to 1.1.30

Signed-off-by: Cian Gallagher <[email protected]>

---------

Signed-off-by: Cian Gallagher <[email protected]>
jbhalodia-slack pushed a commit to jbhalodia-slack/spark-operator that referenced this pull request Oct 4, 2024
* cleanup after upgrade

Signed-off-by: Andrew Chubatiuk <[email protected]>

* pr comments

Signed-off-by: Andrew Chubatiuk <[email protected]>

* feat: add support for custom service labels (kubeflow#1952)

* feat: add support for custom service labels

Signed-off-by: Cian Gallagher <[email protected]>

* chore: correctly format files

Signed-off-by: Cian Gallagher <[email protected]>

* chore: bump chart version to 1.1.30

Signed-off-by: Cian Gallagher <[email protected]>

---------

Signed-off-by: Cian Gallagher <[email protected]>
Signed-off-by: Andrew Chubatiuk <[email protected]>

---------

Signed-off-by: Andrew Chubatiuk <[email protected]>
Signed-off-by: Cian Gallagher <[email protected]>
Co-authored-by: Cian Gallagher <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants