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 shareProcessNamespace flag to podOptions #735

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

gerlowskija
Copy link
Contributor

Boolean flag defaults unset, and is available in the CRD at .spec.customSolrKubeOptions.podOptions.shareProcessNamespace

Boolean flag defaults unset, and is available in the CRD at
`.spec.customSolrKubeOptions.podOptions.shareProcessNamespace`
@@ -42,15 +42,12 @@ annotations:
# Allowed syntax is described at: https://artifacthub.io/docs/topics/annotations/helm/#example
artifacthub.io/changes: |
- kind: added
description: Addition 1
description: SolrClouds now support namespace sharing among pod containers in a pod.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Every time I come back to this project after a lull, I have trouble deciding which chart should hold a given change-entry.

Hopefully I've picked correctly here - I picked the 'solr' chart because the newly-exposed option affects the Solr pods (and not the operator pod itself)

Copy link
Contributor

Choose a reason for hiding this comment

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

Solr Operator, since that is the main chart (you can use the solr operator chart without using the solr chart, but not really the other way around). The Solr chart changeling is only used for changes relating to that helm chart.

@gerlowskija
Copy link
Contributor Author

This PR should allow us to close #716

@@ -137,6 +137,10 @@ type PodOptions struct {
// +optional
ServiceAccountName string `json:"serviceAccountName,omitempty"`

// Should process namespace sharing be enabled on created pods
// +optional
ShareProcessNamespace *bool `json:"shareProcessNamespace,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

Since the default is false, this probably doesn't need to be a pointer. pointer is only used when we want to differentiate the default value of the variable and the default when users leave the field empty.

Copy link
Contributor

@HoustonPutman HoustonPutman left a comment

Choose a reason for hiding this comment

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

this option should be mentioned in the Readme, values.yaml & _custom_option_helpers.tpl of the solr chart, since all the custom options are explicitly listed/set there.

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.

Unable to pass shareProcessNamespace to PodOptions
2 participants