make docs and examples use `istioctl kube-inject` by ayj · Pull Request #87 · istio/istio

ayj · 2017-03-23T23:29:32Z

I verified that demos/apps/bookinfo/README.md works with the istioctl kube-inject method. But I would be happy if anyone else wants to verify in their own setup before its merged.

I wasn't able to get doc/getting-started working, but it doesn't seem to work in my setup with unmodified changes after #86.

I'm not super happy about the need for istio-inject wrapper function but the alternative using istioctl kube-inject isn't great either, e.g.

 istioctl kube-inject \
             --initImage= hub=docker.io/istio/init:2017-03-22-17.30.06 \
             --runtimeImage= hub=docker.io/istio/runtime:2017-03-22-17.30.06
             -f <file> | kubectl apply -f -

We could this if/when we introduce a stable tag in which case we can update kube inject to use that by default and then usage would look like the following:

istioctl kube-inject -f <file> | kubectl apply -f -

This change is

ayj · 2017-03-23T23:38:23Z

cc @sebastienvas, it looks like some of the functions in https://github.com/istio/istio/blob/master/tests/kubeUtils.sh need to be updated to use istioctl kube-inject if we proceed with this approach. In particular, the kubectl create need to be updated to use istioctl kube-inject (see below). I'll wait to make this change until others review the concept.

istioctl kube-inject \
             --initImage= hub=docker.io/istio/init:2017-03-22-17.30.06 \
             --runtimeImage= hub=docker.io/istio/runtime:2017-03-22-17.30.06
             -f <file> | kubectl apply -f -

kyessenov · 2017-03-23T23:39:57Z

What do you think about embedding SHA into istioctl and then using that for default docker image tags?

ayj · 2017-03-23T23:44:53Z

That could work. Downside is users would need to download the latest version of istioctl anytime we identify a new set of stable images vs. making it implicit via versions specified in istio/istio/kubernetes/istioctl-kube-inject or a proper docker tag.

sebastienvas · 2017-03-24T00:07:19Z

Does istioctl kube-inject support namespace for testing ? ie installing everything into one namespace and only listening on one namespace such that we can have multiple test running in one cluster that don t impact each other ?

ayj · 2017-03-24T00:10:53Z

istioctl kube-inject doesn't do anything with namespaces. From the view of kubernetes there should be no difference between the hardcoded injection and kube-inject method.

frankbu · 2017-03-24T01:41:07Z

demos/apps/bookinfo/README.md

+
+   `istio-inject` is a wrapper function around `istioctl
+   kube-inject` which injects the istio runtime proxy into kubernetes
+   resources files. `istio-inject` and `istioctl kube-inject` are documented [here](https://github.com/istio/istio/blob/master/doc/istioctl.md#kube-inject).


better to use relative reference: ../../../doc/istioctl.md#kube-inject

frankbu · 2017-03-24T01:43:45Z

demos/apps/bookinfo/README.md


   ```bash
-   $ kubectl create -f bookinfo-istio.yaml
+   $ source ../../../kubernetes/istioctl-kube-inject.sh


Move this up before the previous step (cd) then it's only source ./kubernetes/istioctl-kube-inject.sh

frankbu · 2017-03-24T02:11:48Z

demos/apps/bookinfo/README.md

+   resources files. `istio-inject` and `istioctl kube-inject` are documented [here](https://github.com/istio/istio/blob/master/doc/istioctl.md#kube-inject).
+
+   ```bash
+   $ isito-inject bookinfo-istio.yaml | kubectl apply -f -


I'm not sure maintaining and using this istio-inject script is worth it. Maybe it would be better to just provide the whole istioctl kube-inject ... command here instead. I think we can hardcode the hub (docker.io/istio) in the command and provide a simple grep/awk command to extract the correct tag from .../.../.../kubernetes/istio-install/istio-manager.yaml. People will just copy-paste whatever we put here anyway.

I expect churn in the pod template we use, so we need to treat each SHA as incompatible with any other SHA. That's why I suggest we hardcode istioctl's SHA into docker image it uses by default. Once we have a stable alpha API, we can start putting alpha tag, and then switch to x.y.z versions.

ayj · 2017-03-24T04:03:38Z

I created a new PR to change istioctl kube-inject to use --tag and --hub flags instead of the full image name. I also changed the default images to point to docker.io/istio/{init,runtime}:2017-03-22-17.30.06. With this change the extra wrapper script shouldn't be necessary.

The normal usage should look like this:

kubectl create -f <(istioctl kube-inject -f bookinfo-istio.yaml)

And using a new tag without updating to the latest istioctl version looks like this:

kubectl create -f <(istioctl kube-inject -f bookinfo-istio.yaml -tag <new-tag>)

frankbu · 2017-03-24T13:59:07Z

This looks pretty good to me now. Couple of small issues:

now that the various yaml files are just plain app definitions, they should probably be renamed (hello-and-proxy.yaml -> hello.yaml, bookinfo-istio.yaml -> bookinfo.yaml, echo-app-proxy.yaml -> echo-app.yaml, etc.).
since the instructions say to use kubectl create -f <(istioctl kube-inject -f bookinfo-istio.yaml) to inject Istio, we need to emphasize that user MUST install and be using the "correct" version of istioctl (How does user confirm this?)

ayj · 2017-03-24T16:15:21Z

@frankbu, re: point #2, user can confirm correct istioctl version with version subcomand, e.g.

$ istioctl version | grep GitRevision
GitRevision: <known-good-sha>

I'll update the docs with a specific revision once new istioctl binaries are uploaded. How is istioctl binary being released? It looks like @esnible uploaded the most recent set of binaries for win/osx/linux.

frankbu · 2017-03-24T16:37:23Z

@ayj: Isn't there more to it than just making sure that the istioctl version is a known good one? Doesn't the istioctl version that we use need to match the tags being used by the version of istio/kuernetes/istio-install/istio-manager.yaml that we are using when starting the istio runtime services?

ayj · 2017-03-24T16:50:39Z

s/<known-good-sha>/<sha-that-corresponds-to-istio-manager-image>

We can select a version of istioctol that corresponds to the image used into istio-manager.yaml. Ideally they would be the same once e2e acceptance test is automated, but for now we can select things manually, which I believe is what we've currently be doing anyway with the hardcoded injection method.

sebastienvas · 2017-03-24T16:57:04Z

@ayj I am planning to put istioctl binary in some istio-io bucket. And have them accessible from gcsweb.

esnible · 2017-03-24T18:17:20Z

An extra line or two of --help for kube-inject with an example of piping to kubectl apply would be very helpful.

Although the istioctl kube-inject output looks correct after piping to kubectl apply the pod.alpha.kubernetes.io/init-containers annotation appears twice. Why? To reproduce:

kubectl run tomcat --image=docker.io/library/tomcat
kubectl get deployment tomcat --output yaml | ./istioctl-osx kube-inject --filename - | kubectl apply -f -
kubectl get deployment tomcat --output yaml

frankbu

I'm still a little worried that people will often trip up because they're using the wrong isitioctl sha, but otherwise LGTM. Maybe in the doc we could suggest that the use the -tag flag and tell them the value can be found with a command something like this: awk '/image:/{print substr($2,25)}' istio-manager.yaml. Not sure if this is really an issue, but I guess we'll find out soon enough

ayj · 2017-03-24T18:41:38Z

re: annotation appearing twice, the same thing happens with the previous hardcoded yaml files.

$ kubectl create -f hello-and-proxy.yaml
$ kubectl get deployment hello -o yaml | grep init-container
        pod.alpha.kubernetes.io/init-containers: '[{"name":"iptables","image":"docker.io/istio/runtime:2017-03-17-22.11.25","resources":{},"imagePullPolicy":"Always","securityContext":{"capabilities":{"add":["NET_ADMIN"]}}}]'
        pod.beta.kubernetes.io/init-containers: '[{"name":"iptables","image":"docker.io/istio/runtime:2017-03-17-22.11.25","resources":{},"imagePullPolicy":"Always","securityContext":{"capabilities":{"add":["NET_ADMIN"]}}}]'

From http://stackissue.com/kubernetes/kubernetes/move-init-container-feature-from-alpha-to-beta-31026.html

In 1.3, an init container is specified with this annotation key
on the pod or pod template: pods.alpha.kubernetes.io/init-containers.

In 1.4, either that key or this key: pods.beta.kubernetes.io/init-containers`,
can be used.

When you GET an object, you will see both annotation keys with the same values.

andraxylia · 2017-03-24T20:39:14Z

kubernetes/README.md

 ## Deploy your apps

+NOTE: Kubernetes admission controller for transparent proxy is not
+implementedyet . Use `istioctl kube-inject` to modify kubernetes


Insert a space between implemented and yet.

- update istioctl binaries to latest - update acceptance test to use kube-inject - fix typo in README.md

istio-testing · 2017-03-24T22:57:12Z

Jenkins job istio/presubmit passed

ayj · 2017-03-24T22:58:59Z

Review status: 0 of 22 files reviewed at latest revision, 4 unresolved discussions.

demos/apps/bookinfo/README.md, line 59 at r1 (raw file):

Previously, frankbu (Frank Budinsky) wrote…

Move this up before the previous step (cd) then it's only source ./kubernetes/istioctl-kube-inject.sh

Done.

demos/apps/bookinfo/README.md, line 64 at r1 (raw file):

Previously, frankbu (Frank Budinsky) wrote…

better to use relative reference: ../../../doc/istioctl.md#kube-inject

Done.

demos/apps/bookinfo/README.md, line 67 at r1 (raw file):

Previously, kyessenov (Kuat) wrote…

I expect churn in the pod template we use, so we need to treat each SHA as incompatible with any other SHA. That's why I suggest we hardcode istioctl's SHA into docker image it uses by default. Once we have a stable alpha API, we can start putting alpha tag, and then switch to x.y.z versions.

Done.

kubernetes/README.md, line 41 at r3 (raw file):

Previously, andraxylia (Andra Cismaru) wrote…

Insert a space between implemented and yet.

Done.

Comments from Reviewable

istio-testing · 2017-03-24T23:12:04Z

Jenkins job istio/presubmit passed

ayj · 2017-03-24T23:18:57Z

Pre-submit is now passing with kube-inject injecting proxy into bookinfo.yaml. I pushed new versions of istioctl binary which should work for now until those can be pulled from GCS bucket.

@frankbu 's comment about mismatched istioctl is noted, so perhaps we wait and see if its a problem and react accordingly? Other than that I think all comments have been addressed. If not, please let me know.

frankbu · 2017-03-27T14:24:20Z

@ayj: fine with me to merge. I'd like to update the README.md to show both before and after (istio-inject) diagrams of the bookinfo components, but if you merge this PR, I can do that as a separate PR afterwords.

* double to protobuf.Duration * remove markdown file * clarify timeunit granularity * fix field names * splitting into smaller protos * add examples for retries * addressing comments * removing api visibility fields

Update CircleCI badge to new repo url

Signed-off-by: Shriram Rajagopalan <rshriram@tetrate.io>

* add ut for util. * fix lint. * add more fields. * apply comments.

…oxy-image Steps for creating custom proxyv2 image

* Strip # part in URL by default and offer a flag to opt-out (#76) * Strip # part in URL by default and offer flag for opt-out. * Small fix. * Small fix. * release 1.10: use case-insensitive match for host field in authz (#74) Same as https://github.com/istio-private/istio/pull/69 Signed-off-by: Yangmin Zhu <ymzhu@google.com> * Update the proxy SHA (#78) * Strip # part in URL by default and offer flag for opt-out. * Small fix. * Small fix. * Update proxy SHA. * Fix a test. * Update Proxy SHA (#79) * add test for request with # character (#84) * Update the base image for 1.10 (#87) * Update istio.deps Co-authored-by: Oliver Liu <yonggangl@google.com> Co-authored-by: Yangmin Zhu <ymzhu@google.com>

@type

* metadata: add initial support for workload metadata discovery This CL adds initial support for a discovery service for an ambient workload metadata filter. The service works over ECDS (in ADS) and is meant to be used with an Ambient Listener filter to provide workload metadata for all workloads being proxied by an Ambient uProxy. At the moment, the schema for Workload Metadata resources is placed in this repo and will need to be duplicated (once OK'd) into the proxy repo. I have tested this with an `istiod` in a test cluster, as follows: test.sh: ``` toJson () { python3 -c ' import sys, yaml, json yml = list(y for y in yaml.safe_load_all(sys.stdin) if y) if len(yml) == 1: yml = yml[0] json.dump(yml, sys.stdout, indent=4) ' } token=$(echo '{"kind":"TokenRequest","apiVersion":"authentication.k8s.io/v1","spec":{"audiences":["istio-ca"], "expirationSeconds":2592000}}' | kubectl create --raw /api/v1/namespaces/default/serviceaccounts/httpbin/token -f - | jq -j '.status.token') request=$(cat request.yaml | toJson) echo "${request}" | grpcurl -d @ -insecure -rpc-header "authorization: Bearer $token" localhost:15012 envoy.service.discovery.v3.AggregatedDiscoveryService/StreamAggregatedResources ``` request.yaml ``` node: id: sidecar~10.100.5.4~httpbin-74fb669cc6-4bjbc.default~default.svc.cluster.local metadata: CONFIG_NAMESPACE: istio-system typeUrl: type.googleapis.com/envoy.config.core.v3.TypedExtensionConfig resourceNames: - ambient.workload_metadata ``` This results in the following response: ``` { "versionInfo": "2022-05-26T18:31:08Z/2", "resources": [ {"@type":"type.googleapis.com/istio.telemetry.wmds.v1.WorkloadMetadataResources","nodeId":"httpbin-74fb669cc6-4bjbc.default","workloadMetadataResources":[{"ipAddresses":["10.100.5.3"],"instanceName":"envoy-als-6798d7c98d-wmtjs","namespaceName":"logs","containers":["envoy-als"],"workloadName":"envoy-als","canonicalName":"envoy-als","canonicalRevision":"latest"},{"ipAddresses":["10.100.5.4"],"instanceName":"httpbin-74fb669cc6-4bjbc","namespaceName":"default","containers":["httpbin","istio-proxy"],"workloadName":"httpbin","canonicalName":"httpbin","canonicalRevision":"v1"},{"ipAddresses":["10.100.5.6"],"instanceName":"httpbin-74fb669cc6-dphf7","namespaceName":"test","containers":["httpbin"],"workloadName":"httpbin","canonicalName":"httpbin","canonicalRevision":"v1"},{"ipAddresses":["10.100.5.2"],"instanceName":"konnectivity-agent-7c84c6c889-rs6q4","namespaceName":"kube-system","containers":["konnectivity-agent"],"workloadName":"konnectivity-agent","canonicalName":"konnectivity-agent","canonicalRevision":"latest"},{"ipAddresses":["10.138.0.62"],"instanceName":"fluentbit-gke-vknzq","namespaceName":"kube-system","containers":["fluentbit","fluentbit-gke"],"workloadName":"fluentbit-gke-vknzq","workloadType":"KUBERNETES_POD","canonicalName":"fluentbit-gke-vknzq","canonicalRevision":"latest"},{"ipAddresses":["10.138.0.62"],"instanceName":"netd-tpms7","namespaceName":"kube-system","containers":["netd"],"workloadName":"netd-tpms7","workloadType":"KUBERNETES_POD","canonicalName":"netd-tpms7","canonicalRevision":"latest"},{"ipAddresses":["10.138.0.62"],"instanceName":"gke-metrics-agent-h9fqj","namespaceName":"kube-system","containers":["gke-metrics-agent"],"workloadName":"gke-metrics-agent-h9fqj","workloadType":"KUBERNETES_POD","canonicalName":"gke-metrics-agent-h9fqj","canonicalRevision":"latest"},{"ipAddresses":["10.138.0.62"],"instanceName":"pdcsi-node-vrjrs","namespaceName":"kube-system","containers":["csi-driver-registrar","gce-pd-driver"],"workloadName":"pdcsi-node-vrjrs","workloadType":"KUBERNETES_POD","canonicalName":"pdcsi-node-vrjrs","canonicalRevision":"latest"},{"ipAddresses":["10.138.0.62"],"instanceName":"gke-metadata-server-59n4x","namespaceName":"kube-system","containers":["gke-metadata-server"],"workloadName":"gke-metadata-server-59n4x","workloadType":"KUBERNETES_POD","canonicalName":"gke-metadata-server-59n4x","canonicalRevision":"latest"},{"ipAddresses":["10.138.0.62"],"instanceName":"kube-proxy-gke-istiocon-test-1-default-pool-194742d5-zayf","namespaceName":"kube-system","containers":["kube-proxy"],"workloadName":"kube-proxy-gke-istiocon-test-1-default-pool-194742d5-zayf","workloadType":"KUBERNETES_POD","canonicalName":"kube-proxy-gke-istiocon-test-1-default-pool-194742d5-zayf","canonicalRevision":"latest"}]} ], "typeUrl": "type.googleapis.com/envoy.config.core.v3.TypedExtensionConfig", "nonce": "geI+YS+L9yI=ae402003-f0ac-4b9f-9044-5929a3f7379a", "controlPlane": { "identifier": "{\"Component\":\"istiod\",\"ID\":\"istiod-76856bc897-wfqfm\",\"Info\":{\"version\":\"1.15-dev\",\"revision\":\"328d68d3a601fd923e86fdd3e8a8c311def52a0c-dirty\",\"golang_version\":\"go1.17.2\",\"status\":\"Modified\",\"tag\":\"\"}}" } } ``` In the future, we will probably want to refactor the ECDS handling code and also maybe move ECDS to a proper separate service endpointto avoid polluting the other proxy configurations. There are several TODOs throughout the existing code that serve as indications of aareas that will need to be improved once we have a proper filter defined in the proxy. * address review comments * gofumpt * more gofumpt

Added support for specifying the port number to use for the node port for the Jaeger query service

…ase-1.22-merge_upstream_istio_release_1_22-b34c6a22 Automator: merge upstream changes to openshift-service-mesh/istio@release-1.22

ayj requested review from andraxylia, esnible and frankbu March 23, 2017 23:29

googlebot added the cla: yes label Mar 23, 2017

make docs and examples use istioctl kube-inject

b6773ea

frankbu reviewed Mar 24, 2017

View reviewed changes

updates based on comments

d07dfc7

remove "proxy" from yaml resource file names

bb493d0

frankbu approved these changes Mar 24, 2017

View reviewed changes

andraxylia approved these changes Mar 24, 2017

View reviewed changes

ayj added 4 commits March 24, 2017 15:13

misc updates

41b1e5c

- update istioctl binaries to latest - update acceptance test to use kube-inject - fix typo in README.md

Merge branch 'master' of github.com:istio/istio into use-kube-inject

19b8c1f

s/istioctl/${ISTIOCLI}/

380d9b0

refresh istioctl binary

d7fed11

ayj added 2 commits March 24, 2017 16:03

remove out-of-date reference to istioctl-kube-inject.sh

5dc0029

remove out of date kube-inject --help output

6f32d59

ayj merged commit 2bd73c5 into istio:master Mar 27, 2017

ayj deleted the use-kube-inject branch March 27, 2017 16:21

kyessenov pushed a commit to kyessenov/istio that referenced this pull request Aug 13, 2018

Support per-request transport for Check. (istio#87)

6c4965e

rajusharma pushed a commit to rajusharma/istio that referenced this pull request Jul 2, 2019

Merge pull request istio#87 from c0d1ngm0nk3y/update-circleci-badge

4477469

Update CircleCI badge to new repo url

vikaschoudhary16 pushed a commit to vikaschoudhary16/istio that referenced this pull request Oct 11, 2019

Bug fixes based on TCC testing (istio#87)

b9e9bd7

Signed-off-by: Shriram Rajagopalan <rshriram@tetrate.io>

howardjohn pushed a commit to howardjohn/istio that referenced this pull request Jan 12, 2020

add ut for util. (istio#87)

cb9a720

* add ut for util. * fix lint. * add more fields. * apply comments.

deva26 pushed a commit to deva26/istio that referenced this pull request Jun 24, 2020

Merge pull request istio#87 from vikaschoudhary16/steps-for-custom-pr…

1eb00d4

…oxy-image Steps for creating custom proxyv2 image

jacob-delgado pushed a commit that referenced this pull request Aug 24, 2021

Update the base image for 1.10 (#87)

830ecc9

antonioberben pushed a commit to antonioberben/istio that referenced this pull request Jan 29, 2024

Merge pull request istio#87 from marcusportmann/master

cf8d895

Added support for specifying the port number to use for the node port for the Jaeger query service

Conversation

ayj commented Mar 23, 2017 • edited by mandarjog Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ayj commented Mar 23, 2017

Uh oh!

kyessenov commented Mar 23, 2017

Uh oh!

ayj commented Mar 23, 2017

Uh oh!

sebastienvas commented Mar 24, 2017

Uh oh!

ayj commented Mar 24, 2017

Uh oh!

frankbu Mar 24, 2017

Choose a reason for hiding this comment

Uh oh!

frankbu Mar 24, 2017

Choose a reason for hiding this comment

Uh oh!

frankbu Mar 24, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kyessenov Mar 24, 2017

Choose a reason for hiding this comment

Uh oh!

ayj commented Mar 24, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

frankbu commented Mar 24, 2017

Uh oh!

ayj commented Mar 24, 2017

Uh oh!

frankbu commented Mar 24, 2017

Uh oh!

ayj commented Mar 24, 2017

Uh oh!

sebastienvas commented Mar 24, 2017

Uh oh!

esnible commented Mar 24, 2017

Uh oh!

frankbu left a comment

Choose a reason for hiding this comment

Uh oh!

ayj commented Mar 24, 2017

Uh oh!

andraxylia Mar 24, 2017

Choose a reason for hiding this comment

Uh oh!

istio-testing commented Mar 24, 2017

Uh oh!

ayj commented Mar 24, 2017

Uh oh!

istio-testing commented Mar 24, 2017

Uh oh!

ayj commented Mar 24, 2017

Uh oh!

frankbu commented Mar 27, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

ayj commented Mar 23, 2017 •

edited by mandarjog

Loading

frankbu Mar 24, 2017 •

edited

Loading

ayj commented Mar 24, 2017 •

edited

Loading