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

Add additionalServiceOptions #398

Merged
merged 6 commits into from
Mar 29, 2021
Merged

Add additionalServiceOptions #398

merged 6 commits into from
Mar 29, 2021

Conversation

burmanm
Copy link
Contributor

@burmanm burmanm commented Mar 8, 2021

Allows defining additional labels and/or annotations to generated services. At the moment it only creates it to the main-service (--service) - do we want the same replicated to cluster2-seed-service and cluster2-dc2-all-pods-service as well ?

Fixes #293 and k8ssandra/k8ssandra#478

Copy link
Collaborator

@jsanda jsanda left a comment

Choose a reason for hiding this comment

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

@burmanm not sure if you have seen the latest comments in #293 but the request is to be able to specify labels and annotation per service.

Copy link
Collaborator

@jsanda jsanda left a comment

Choose a reason for hiding this comment

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

@burmanm The changes look good from my perspective. The only remaining request I have is a minor doc update to indicate that user-supplied labels will overwrite those created by cass-operator.

Copy link
Collaborator

@jsanda jsanda left a comment

Choose a reason for hiding this comment

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

Changes LGTM.

@jimdickinson can you review please?

@@ -181,6 +181,9 @@ type CassandraDatacenterSpec struct {

// Container image for the log tailing sidecar container.
SystemLoggerImage string `json:"systemLoggerImage,omitempty"`

// AdditionalServiceConfig allows to define additional parameters that are included in the created Services. Note, user can override values set by cass-operator and doing so could break cass-operator functionality.
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍 , but we'll probably have to document which labels they should avoid, somewhere

Copy link
Collaborator

Choose a reason for hiding this comment

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

@burmanm can you document the labels that need to be avoided? I think there are only a handful, in which case maybe just list them in the comments here.

Copy link
Collaborator

@jimdickinson jimdickinson left a comment

Choose a reason for hiding this comment

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

👍 looks really good! just addAdditionalOptions before we AddHashAnnotation

@@ -181,6 +181,9 @@ type CassandraDatacenterSpec struct {

// Container image for the log tailing sidecar container.
SystemLoggerImage string `json:"systemLoggerImage,omitempty"`

// AdditionalServiceConfig allows to define additional parameters that are included in the created Services. Note, user can override values set by cass-operator and doing so could break cass-operator functionality.
Copy link
Collaborator

Choose a reason for hiding this comment

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

@burmanm can you document the labels that need to be avoided? I think there are only a handful, in which case maybe just list them in the comments here.

Copy link
Collaborator

@jsanda jsanda left a comment

Choose a reason for hiding this comment

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

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow for specifying additional labels on client service
3 participants