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: Allow to set environment variables into Eclipse Che containers #1468

Merged
merged 4 commits into from
Aug 6, 2022

Conversation

tolusha
Copy link
Contributor

@tolusha tolusha commented Aug 4, 2022

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

What does this PR do?

Allow to set environment variables into Eclipse Che containers

Screenshot/screencast of this PR

N/A

What issues does this PR fix or reference?

eclipse-che/che#21605

How to test this PR?

  1. Generate chectl templates:
make gen-chectl-tmpl TARGET=/tmp/operator-resources 
  1. Deploy Eclipse Che
chectl server:deploy --platform=minikube --templates /tmp/operator-resources --che-operator-image=quay.io/abazko/operator:21605
  1. Patch CheCluster CR
kubectl patch checluster/eclipse-che --patch '{"spec": {"components": {"dashboard": {"deployment": {"containers": [{"env": [{"name": "KUBERNETES_PORT_443_TCP", "value": "tcp://10.96.0.1:440"}]}]}}}}}' --type=merge -n eclipse-che
  1. Wait some time until a new dashboard pod is run
  2. Check env var
kubectl exec deploy/che-dashboard -n eclipse-che -- printenv KUBERNETES_PORT_443_TCP

PR Checklist

As the author of this Pull Request I made sure that:

Reviewers

Reviewers, please comment how you tested the PR when approving it.

tolusha added 2 commits August 4, 2022 15:16
Signed-off-by: Anatolii Bazko <abazko@redhat.com>
Signed-off-by: Anatolii Bazko <abazko@redhat.com>
@openshift-ci
Copy link

openshift-ci bot commented Aug 4, 2022

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: tolusha

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
Copy link
Contributor Author

tolusha commented Aug 5, 2022

/retest

@codecov
Copy link

codecov bot commented Aug 5, 2022

Codecov Report

Merging #1468 (5864bc5) into main (7f051fa) will decrease coverage by 0.03%.
The diff coverage is 41.66%.

❗ Current head 5864bc5 differs from pull request most recent head 91b3ab1. Consider uploading reports for the commit 91b3ab1 to get more accurate results

@@            Coverage Diff             @@
##             main    #1468      +/-   ##
==========================================
- Coverage   60.36%   60.33%   -0.04%     
==========================================
  Files          74       74              
  Lines        6275     6287      +12     
==========================================
+ Hits         3788     3793       +5     
- Misses       2119     2126       +7     
  Partials      368      368              
Impacted Files Coverage Δ
api/v2/checluster_types.go 35.00% <ø> (ø)
api/v2/zz_generated.deepcopy.go 0.00% <0.00%> (ø)
pkg/common/utils/utils.go 30.00% <0.00%> (-0.77%) ⬇️
pkg/deploy/deployment.go 57.51% <100.00%> (+1.12%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Contributor

@karatkep karatkep left a comment

Choose a reason for hiding this comment

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

Thank you @tolusha, I commented minor change, but in overall this PR is great!

@@ -102,6 +102,16 @@ func GetEnv(envs []corev1.EnvVar, name string) string {
return ""
}

func FindEnv(envs []corev1.EnvVar, name string) int {
Copy link
Contributor

Choose a reason for hiding this comment

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

A minor remark: can we rename this method just to be more specific? For example:

// Index returns the index of the first element of envs with requested name, or -1 if envs has no such element.
func Index(envs []corev1.EnvVar, name string) int {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@karatkep
Sure

tolusha added 2 commits August 6, 2022 14:49
Signed-off-by: Anatolii Bazko <abazko@redhat.com>
Signed-off-by: Anatolii Bazko <abazko@redhat.com>
@openshift-ci
Copy link

openshift-ci bot commented Aug 6, 2022

@tolusha: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/v11-che-behind-proxy 91b3ab1 link true /test v11-che-behind-proxy
ci/prow/v11-devworkspace-happy-path 91b3ab1 link true /test v11-devworkspace-happy-path

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

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.

3 participants