Skip to content

Mesh ProxyConfig: Add envoy metadata#902

Closed
drichelson wants to merge 1 commit intoistio:release-1.1from
drichelson:addEnvoyMetadata
Closed

Mesh ProxyConfig: Add envoy metadata#902
drichelson wants to merge 1 commit intoistio:release-1.1from
drichelson:addEnvoyMetadata

Conversation

@drichelson
Copy link
Copy Markdown

@drichelson drichelson commented Apr 16, 2019

This continues to address istio/istio#11984 and is a follow-on to istio/istio#12189

We're adding the node metadata to this struct so it can be easily plugged in to the bootstrap config when pilot-agent starts.

@googlebot googlebot added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label Apr 16, 2019
@istio-testing
Copy link
Copy Markdown
Collaborator

istio-testing commented Apr 16, 2019

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: drichelson
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: costinm

If they are not already assigned, you can assign the PR to them by writing /assign @costinm in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details 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

@istio-testing
Copy link
Copy Markdown
Collaborator

istio-testing commented Apr 16, 2019

Hi @drichelson. Thanks for your PR.

I'm waiting for a istio member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

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.

@drichelson drichelson mentioned this pull request Apr 16, 2019
Merged
@rshriram
Copy link
Copy Markdown
Member

rshriram commented Apr 17, 2019

/ok-to-test

@istio-testing istio-testing added ok-to-test Set this label allow normal testing to take place for a PR not submitted by an Istio org member. and removed needs-ok-to-test labels Apr 17, 2019
rshriram
rshriram reviewed Apr 17, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@rshriram rshriram left a comment

Choose a reason for hiding this comment

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

i dont think you need this. if in your helm templates or sidecar injection config, you supply environment variables of form ISTIO_META_BLAH, Pilot agent will extract them and automatically inject node metadata into envoy before starting it.

@drichelson
Copy link
Copy Markdown
Author

drichelson commented Apr 17, 2019

@rshriram Please see istio PR for a little more context: https://github.com/istio/istio/pull/13367/files
We already have cert file paths as args in pilot-agent. They are used for both watching custom cert paths + sending metadata to pilot-discovery. Are you suggesting we rework the cert file watcher config code to read from env vars?

@rshriram
Copy link
Copy Markdown
Member

rshriram commented Apr 24, 2019

Closing as the issue has been resolved in a different way. Thanks @drichelson for your patience!

@rshriram rshriram closed this Apr 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@frankbu frankbu Awaiting requested review from frankbu

@geeknoid geeknoid Awaiting requested review from geeknoid

1 more reviewer

@rshriram rshriram rshriram left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. ok-to-test Set this label allow normal testing to take place for a PR not submitted by an Istio org member.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@drichelson @istio-testing @rshriram @googlebot