CSPL-633 Support extraENV in CR spec #325

jryb · 2021-04-30T16:51:23Z

Problem

Splunk operator developers need a method to add additional env variables to the Splunk instance pods for debugging and Splunk app installation. The operator needs to provide them with a method of defining these environment variables in the CRD so that they are propagated to all created Splunk instances when deployed.

Solution

Allow users to specify additional env variables to be passed down to the Splunk instance
pods via the CRD with the following config:

  kind: Standalone
  metadata:
    name: sa
  spec:
    extraEnv:
    - name: SOME_COOL_ENV_VAR
      value: "stuff"
  - name: ANSIBLE_EXTRA_FLAGS
    value: "-vvv"

These environment variables will be passed and set in the env of each Splunk instances
created by applying this CRD.

Added additional unit tests to validate that the extraEnv parameters are passed to the
env of the created Splunk instance containers.

Tests

Along with adding unit tests, this functionality can be verified functionally as well. Validate that when adding ANSIBLE_EXTRA_FLAGS to the extraEnv: section, Ansible prints out verbose debugs:

apiVersion: enterprise.splunk.com/v1beta1
kind: Standalone
metadata:
  name: sa
  finalizers:
  - enterprise.splunk.com/delete-pvc
spec:
  replicas: 1
  extraEnv:
  - name: SOME_COOL_ENV_VAR
    value: "stuff"
  - name: ANSIBLE_EXTRA_FLAGS
    value: "-vvv"

Check kubectl describe:

$ kdes pod/splunk-sa-standalone-0
Name:         splunk-sa-standalone-0
Namespace:    default
...
Controlled By:  StatefulSet/splunk-sa-standalone
Containers:
  splunk:
...
    Environment:
      ANSIBLE_EXTRA_FLAGS:                -vvv
      SOME_COOL_ENV_VAR:                  stuff

Validate with ansible logs:

<snip>
included: /opt/ansible/roles/splunk_common/tasks/licenses/add_license.yml for localhost
Friday 30 April 2021  16:37:10 +0000 (0:00:00.114)       0:00:51.036 ********** 
Friday 30 April 2021  16:37:10 +0000 (0:00:00.108)       0:00:51.145 ********** 
Using module file /usr/lib/python3.7/site-packages/ansible/modules/stat.py
Pipelining is enabled.
<localhost> ESTABLISH LOCAL CONNECTION FOR USER: splunk
<localhost> EXEC /bin/sh -c '/usr/bin/python && sleep 0'

TASK [splunk_common : Ensure license path] *************************************
task path: /opt/ansible/roles/splunk_common/tasks/licenses/add_license.yml:15
ok: [localhost] => {
    "changed": false,
    "invocation": {
        "module_args": {
            "checksum_algorithm": "sha1",
            "follow": false,
            "get_attributes": true,
            "get_checksum": true,
            "get_md5": false,
            "get_mime": true,
            "path": "splunk.lic"
        }
    },
    "stat": {
        "exists": false
    }
}
Friday 30 April 2021  16:37:10 +0000 (0:00:00.752)       0:00:51.897 ********** 
Friday 30 April 2021  16:37:10 +0000 (0:00:00.083)       0:00:51.981 ********** 
META: ran handlers
<snip>

pkg/splunk/enterprise/indexercluster.go

pkg/splunk/enterprise/clustermaster.go

vebken-splunk · 2021-04-30T23:11:53Z

I believe you would need to run "make generate" so that the CRDs themselves are updated with extraEnv in this PR as well, right?

jryb · 2021-05-03T17:07:37Z

I believe you would need to run "make generate" so that the CRDs themselves are updated with extraEnv in this PR as well, right?

Yeah I need to run make generate. I did however it created a lot of white space/spacing diffs. So I'm trying a few things to make sure the CRD changes that are generated are isolated to these changes only.

jryb · 2021-05-03T21:33:07Z

Squashed commits

jryb · 2021-05-03T21:36:32Z

I believe you would need to run "make generate" so that the CRDs themselves are updated with extraEnv in this PR as well, right?

Yeah I need to run make generate. I did however it created a lot of white space/spacing diffs. So I'm trying a few things to make sure the CRD changes that are generated are isolated to these changes only.

Fixed by using yq v3.2.1

sgontla · 2021-05-12T18:05:55Z

pkg/splunk/enterprise/configuration.go

+	// Add extraEnv from the CommonSplunkSpec config to the extraEnv variable list
+	for _, envVar := range spec.ExtraEnv {
+		extraEnv = append(extraEnv, envVar)
+	}
+


Looking at the CRD changes seems like []corev1.EnvVar, is supporting multiple ways of specifying the env variables to source from(configMap, secret object, etc..). Wondering if we need to deal with them the same way, like reading those resource objects, and populating them as part of the extraEnv here?

Also, what happens if somebody specifies an environment source, instead of the value, and the source is not created/available. Do we need to do a sanity check on those cases?

Since the extraEnv is the same []corev1.EnvVar object, it will behave the same as all the other EnvVar objects. Here we are just appending the ExtraEnv section to the env vars already created. If EnvVarSource is used and the source doesn't exist, this should throw an error when creating the pod.
Applied CR:

apiVersion: enterprise.splunk.com/v1beta1 kind: Standalone metadata: name: sa finalizers: - enterprise.splunk.com/delete-pvc spec: replicas: 1 extraEnv: - name: ANSIBLE_EXTRA_FLAGS value: "-vvv" - name: FROM_SOURCE valueFrom: secretKeyRef: name: doesntexistsecret key: awsSecretAccessKey

Error:

splunk-sa-standalone-0 0/1 CreateContainerConfigError 0 101s 10.244.0.22 kind-control-plane <none> <none> ... Events: Type Reason Age From Message ---- ------ ---- ---- ------- Normal Scheduled 13s default-scheduler Successfully assigned default/splunk-sa-standalone-0 to kind-control-plane Normal Pulled 1s (x3 over 13s) kubelet Container image "docker.io/splunk/splunk:8.1.3" already present on machine Warning Failed 1s (x3 over 13s) kubelet Error: secret "doesntexistsecret" not found

I'm not sure we really need to this prior to apply since its already handled by kubectl.

Thanks Jeff.
Doing it prior may help in the case of incremental config change, where we destroy an existing Pod, before starting a pod with the missing resource, which may be a rare case :-)

smohan-splunk · 2021-05-12T19:58:05Z

deploy/crds/enterprise.splunk.com_clustermasters_crd.yaml

@@ -943,7 +1047,6 @@ spec:
                              x-kubernetes-int-or-string: true
                          required:
                          - port
-                          - protocol


Is this removed because of the newer SDK?

Yes, I assume so. I didn't touch this config, just ran make generate and it threw out this diff.

Having to re-add the protocol field is a limitation of the workaround to kubernetes/kubernetes#91395 at the moment that we have to do anytime we generate CRDs.
I have been looking into a better solution to automate this process or workaround the issue altogether, I can open a CSPL to formally track this.

Other projects that have hit the issue (kubernetes-sigs/controller-tools#444) mention upgrading their controller-gen version works around the issue, but I don't believe we use controller-gen to generate the crds. There are some other ideas mentioned here kubernetes/kubernetes#91395, but the automated ones seem to require modifying k8s api code.

jryb · 2021-05-14T19:31:40Z

Manually readded https://github.com/splunk/splunk-operator/pull/331/commits back, updated to latest and squashed.

Allow users to specify additional env variables to be passed down to the Splunk instance pods via the CRD with the following config: kind: Standalone metadata: name: sa spec: extraEnv: - name: SOME_ENV_VAR value: "value1" - name: MORE_ENV_VARS value: "value2" These environment variables will be passed and set in the evn of each Splunk instances created by applying this CRD. Added additional unit tests to validate that the `extraEnv` parameters are passed to the `env` of the created Splunk instance containers.

jryb requested review from akondur and smohan-splunk April 30, 2021 16:51

akondur reviewed Apr 30, 2021

View reviewed changes

pkg/splunk/enterprise/indexercluster.go Outdated Show resolved Hide resolved

pkg/splunk/enterprise/clustermaster.go Outdated Show resolved Hide resolved

akondur approved these changes Apr 30, 2021

View reviewed changes

jryb closed this May 3, 2021

jryb deleted the bugfix/CSPL-633_extraEnv branch May 3, 2021 21:27

jryb reopened this May 3, 2021

jryb force-pushed the bugfix/CSPL-633_extraEnv branch from e60cd5c to 92610a8 Compare May 3, 2021 21:35

jryb closed this May 3, 2021

jryb reopened this May 3, 2021

jryb force-pushed the bugfix/CSPL-633_extraEnv branch from d3cfb4d to a7da7de Compare May 3, 2021 23:31

jryb force-pushed the bugfix/CSPL-633_extraEnv branch from a7da7de to a236de3 Compare May 12, 2021 17:33

sgontla reviewed May 12, 2021

View reviewed changes

sgontla approved these changes May 12, 2021

View reviewed changes

smohan-splunk approved these changes May 12, 2021

View reviewed changes

smohan-splunk requested a review from vebken-splunk May 12, 2021 20:26

jryb requested a review from pdhanoya-splunk May 13, 2021 17:45

jryb force-pushed the bugfix/CSPL-633_extraEnv branch from 6c96309 to 042e7d1 Compare May 14, 2021 19:29

vebken-splunk approved these changes May 14, 2021

View reviewed changes

pdhanoya-splunk approved these changes May 14, 2021

View reviewed changes

jryb force-pushed the bugfix/CSPL-633_extraEnv branch 2 times, most recently from 0f4393a to b5c9ae3 Compare May 17, 2021 17:33

jryb force-pushed the bugfix/CSPL-633_extraEnv branch from 5248185 to dd7622e Compare May 17, 2021 21:53

smohan-splunk merged commit 26ef4df into develop May 18, 2021

jryb mentioned this pull request May 18, 2021

support extraENV in spec #184

Closed

pogdin mentioned this pull request Jun 2, 2021

Feature Request: Allow setting environment variables on CRD objects #363

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CSPL-633 Support extraENV in CR spec #325

CSPL-633 Support extraENV in CR spec #325

jryb commented Apr 30, 2021 •

edited

Loading

vebken-splunk commented Apr 30, 2021

jryb commented May 3, 2021

jryb commented May 3, 2021

jryb commented May 3, 2021

sgontla May 12, 2021 •

edited

Loading

jryb May 12, 2021

sgontla May 12, 2021

smohan-splunk May 12, 2021

jryb May 12, 2021

vebken-splunk May 14, 2021

jryb commented May 14, 2021

CSPL-633 Support extraENV in CR spec #325

CSPL-633 Support extraENV in CR spec #325

Conversation

jryb commented Apr 30, 2021 • edited Loading

Problem

Solution

Tests

vebken-splunk commented Apr 30, 2021

jryb commented May 3, 2021

jryb commented May 3, 2021

jryb commented May 3, 2021

sgontla May 12, 2021 • edited Loading

Choose a reason for hiding this comment

jryb May 12, 2021

Choose a reason for hiding this comment

sgontla May 12, 2021

Choose a reason for hiding this comment

smohan-splunk May 12, 2021

Choose a reason for hiding this comment

jryb May 12, 2021

Choose a reason for hiding this comment

vebken-splunk May 14, 2021

Choose a reason for hiding this comment

jryb commented May 14, 2021

jryb commented Apr 30, 2021 •

edited

Loading

sgontla May 12, 2021 •

edited

Loading