Set the serviceCluster namespace based on env var, to also support sp…#11521
Set the serviceCluster namespace based on env var, to also support sp…#11521objectiser wants to merge 1 commit intoistio:masterfrom
Conversation
…ecifying namespace on cli after kubeinject
|
@objectiser: The following test failed, say
DetailsInstructions 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. |
|
Tested against bookinfo example, in a @douglas-reid Should this be moved to 1.1 branch? |
|
@objectiser good question. I don't have an answer, unfortunately. My guess is that this will have to wait until 1.1.1 or 1.2, but the details on the upcoming release cycle changes haven't been announced. @wenchenglu any insight? |
|
this sounds a nice enhancement. Will that env var set to "default" namespace by default? @douglas-reid is there any particular concern this PR might break anything? otherwise I am fine to merge it into 1.1. |
|
Can we merge this into Istio 1.1 branch too? |
|
I don't believe this will break anything. |
|
per chat with @douglas-reid, yes, we think that is a good change to go in 1.1 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: objectiser, wenchenglu If they are not already assigned, you can assign the PR to them by writing The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
@wenchenglu @douglas-reid PR created on 1.1 branch #11587 - which has now been merged. |
|
@objectiser should we wait until the 1.1 changes are merged back and then close this? |
|
Or could close now if you would prefer.
I have patchy network access for next two weeks in case you wonder why I am
not responding.
…On Fri, 15 Feb 2019, 19:24 Douglas Reid ***@***.*** wrote:
@objectiser <https://github.com/objectiser> should we wait until the 1.1
changes are merged back and then close this?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#11521 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAKC0vuW4CkYOJKmIkbaSECIwscefOkMks5vNwlegaJpZM4ajFwm>
.
|
|
This pull request has been automatically marked as stale because it has not had activity in the last 2 weeks. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
|
@objectiser: PR needs rebase. DetailsInstructions 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. |
istio-testing
added
the
needs-rebase
6 participants
…ecifying namespace on cli after kube-inject
If the user deploys the app by specifying the namespace on the command line,
then currently this is after the sidecar injection config has been added, so the
serviceClusterparameter has the wrong value (e.g. associated withdefaultnamespace).By using the
${POD_NAMESPACE}env var it resolves the serviceCluster name (and namespace) at a later stage, taking into account namespace specified on the command line.