Skip to content

Add note for reroute-virtual-interfaces usage with dind #3494

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

Merged
merged 2 commits into from
May 2, 2025

Conversation

sridhargaddam
Copy link
Contributor

No description provided.

Signed-off-by: Sridhar Gaddam <sgaddam@redhat.com>
@sridhargaddam sridhargaddam requested a review from a team as a code owner April 19, 2025 16:27
@istio-policy-bot
Copy link

😊 Welcome @sridhargaddam! This is either your first contribution to the Istio api repo, or it's been
a while since you've been here.

You can learn more about the Istio working groups, Code of Conduct, and contribution guidelines
by referring to Contributing to Istio.

Thanks for contributing!

Courtesy of your friendly welcome wagon.

@istio-testing istio-testing added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Apr 19, 2025
@sridhargaddam sridhargaddam added the release-notes-none Indicates a PR that does not require release notes. label Apr 19, 2025
@sridhargaddam
Copy link
Contributor Author

/test release-notes

@sridhargaddam
Copy link
Contributor Author

@@ -578,6 +578,7 @@ annotations:
featureStatus: Alpha
description: |
A comma separated list of virtual interfaces whose inbound traffic will be unconditionally treated as outbound. This allows workloads using virtualized networking (kubeVirt, VMs, docker-in-docker, etc) to function correctly with mesh traffic capture.
Note: In the case of docker-in-docker containers, the default Docker bridge name is not fixed. To have a predictable name, you can configure the Docker option `com.docker.network.bridge.name` with a fixed value and use that name in the annotation.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Note: In the case of docker-in-docker containers, the default Docker bridge name is not fixed. To have a predictable name, you can configure the Docker option `com.docker.network.bridge.name` with a fixed value and use that name in the annotation.
Note: When using docker-in-docker container, the default bridge interface name is typically `docker0`. However, custom networks (often used with docker compose) are assigned a randomized interface name. To have a predictable name, you can configure the Docker option `com.docker.network.bridge.name` with a fixed value and use that name in the annotation.

Choose a reason for hiding this comment

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

I'd suggest putting in a full docker-compose example... eg:

networks:
  custom_bridge:
    driver: bridge
    driver_opts:
      com.docker.network.bridge.name: "istio-docker0" <<  point out that this needs to be done in conjunction with the annotation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you @howardjohn.

@diranged it looks a bit too verbose for a note. But if John/others, feel that including the full networks section is fine, I'm happy to update the PR.

Copy link
Member

Choose a reason for hiding this comment

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

I agree it is a bit verbose, could we host the special instruction as the example in the doc instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried searching the current docs but couldn't find an appropriate section where this note could be added.

Copy link
Member

@howardjohn howardjohn left a comment

Choose a reason for hiding this comment

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

LGTM! thanks

Signed-off-by: Sridhar Gaddam <sgaddam@redhat.com>
@linsun
Copy link
Member

linsun commented Apr 22, 2025

lgtm - proposed change from John.

Copy link
Contributor

@costinm costinm left a comment

Choose a reason for hiding this comment

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

Seems reasonable - since we mention already DIND.

@istio-testing istio-testing merged commit d98ae9c into istio:master May 2, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes-none Indicates a PR that does not require release notes. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants