[release-5.9]LOG-6963:Map cluster-wide proxy env variable for compatibility with Vector expectations #3032
Conversation
|
This should most likely be fixed in 6.2 (and master), rather than 5.9. Also, I'm not seeing how this is related to fixing the '@' issue in: https://issues.redhat.com/browse/LOG-6963 |
|
Shouldn't we be merging this fix into master and release-6.2 first. Is there a way we can resolve this in v6, then tell them to upgrade if they complain about not fixing in v5? |
Vector just don't picked up env value in lowercase, not related directly to the Something similar was here: https://issues.redhat.com/browse/LOG-4900 |
Maybe yes, original issue opened for 5.9 thats why i did it firstly |
| # Remap lowercase to uppercase if it's set | ||
| if [ -z "${HTTP_PROXY:-}" ] && [ -n "${http_proxy:-}" ]; then | ||
| export HTTP_PROXY="$http_proxy" | ||
| echo HTTP_PROXY="$HTTP_PROXY" |
There was a problem hiding this comment.
If we are going to add a "log" statement then lets say what we are doing... "Setting HTTP_PROXY=...."
There was a problem hiding this comment.
Is the proxy url actually something we want to put into a log? Couldn't it contain user/pwd?
| Expect(framework.DeployWithVisitor(envVisitor)).To(BeNil()) | ||
| out, err := framework.ReadCollectorLogs() | ||
| Expect(err).To(BeNil()) | ||
| Expect(out).To(ContainSubstring(`HTTP_PROXY=http://my_proxy:8080`)) |
There was a problem hiding this comment.
I believe a more accurate assertion would be to exec into the pod, get the env vars, and check explicitly for the environment variable and value
There was a problem hiding this comment.
yes, get the env vars will be better but with here var sets only in the process running that script, It does not persist globally in the container environment.
openshift-ci
bot
added
the
do-not-merge/hold
|
/retest |
vparfonov
changed the title
openshift-ci
bot
added
the
do-not-merge/work-in-progress
…ectations Signed-off-by: Vitalii Parfonov <[email protected]>
vparfonov
changed the title
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
openshift-ci-robot
added
the
jira/valid-reference
|
@vparfonov: This pull request references LOG-6963 which is a valid jira issue. In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
@vparfonov: This pull request references LOG-6963 which is a valid jira issue. In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jcantrill, vparfonov 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 |
openshift-ci
bot
added
the
approved
|
@vparfonov: all tests passed! 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-sigs/prow repository. I understand the commands that are listed here. |
Description
Adding cluster-wide proxy automatically inject the environment variable, to support
Fluentdcollector they was lowercased tohttp_proxy,https_proxyandno_proxy. In the same time, the Vector expects the variable to be namedHTTP_PROXY,HTTPS_PROXYandNO_PROXY(in uppercase) [1]. This mismatch causes the Vector not picked up proxy settings.This PR adds a mechanism to support both collectors in the Vector value will be remapped according to current collector expectations.
[1] https://github.com/vectordotdev/vector/blob/f5b9265335c9df7bcbd4a57046738fb5e87a67c1/lib/vector-core/src/config/proxy.rs#L121
/cc @Clee2691 @cahartma
/assign @jcantrill
Links