Skip to content

Conversation

devodev
Copy link
Contributor

@devodev devodev commented Oct 22, 2024

Implement automatic (opt-in) port-forwarding capability in Hubble-cli.

As proposed in #35353, this uses native port-forwarding by introducing a new high-level PortForwarder type to add automatic port-forwarding to all commands that require a GRPC connection to hubble-relay.

Fixes: #34935

Add support for automatic port-forwarding in Hubble CLI
Replace kubectl-based port-forwarding with native implementation in Cilium CLI

This also updates hubble port-forwarding commands in cilium-cli to replace the kubectl-based implementation, but we can also do that in a different PR if desired. Added bonus is we don't need to sleep in a goroutine before opening the browser to mitigate the kubectl spawn process delay, nice.

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Oct 22, 2024
@github-actions github-actions bot added hubble-cli PRs or GitHub issues related with hubble-cli cilium-cli This PR contains changes related with cilium-cli kind/community-contribution This was a contribution made by a community member. labels Oct 22, 2024
@devodev devodev force-pushed the pr/devodev/hubble-auto-port-forward-native branch from 1394e17 to 3a0885f Compare October 22, 2024 16:01
@devodev devodev marked this pull request as ready for review October 22, 2024 16:04
@devodev devodev requested review from a team as code owners October 22, 2024 16:04
Copy link
Contributor

@chancez chancez left a comment

Choose a reason for hiding this comment

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

LGTM overall. Mostly nitpicks so nothing blocking and the functional changes are good. Thanks for keeping things broken into multiple commits.

@michi-covalent
Copy link
Contributor

/test

@michi-covalent
Copy link
Contributor

@chancez i guess we can remove dont-merge/blocked label now?

@devodev devodev force-pushed the pr/devodev/hubble-auto-port-forward-native branch from 0983d85 to 24a858c Compare October 30, 2024 15:33
@devodev
Copy link
Contributor Author

devodev commented Oct 30, 2024

Fixed the go mod/vendor check with my latest force-push

@michi-covalent
Copy link
Contributor

/test

@devodev
Copy link
Contributor Author

devodev commented Oct 30, 2024

The hubble-cli compile test started failing on 32bit platform after the latest rebase. Opened a PR for a fix attempt.

@chancez chancez removed the dont-merge/blocked Another PR must be merged before this one. label Oct 30, 2024
…dutils

Signed-off-by: Alexandre Barone <abalexandrebarone@gmail.com>
Add a new type that provides helper methods for kubernetes
port-forwarding which is not linked to the cilium-cli k8s
client to allow re-use.

Signed-off-by: Alexandre Barone <abalexandrebarone@gmail.com>
Add a new helper method that finds the best candidate
pod for port-forwrding using a service reference.

Signed-off-by: Alexandre Barone <abalexandrebarone@gmail.com>
Add the capability to connect to hubble relay by creating
a port-forward to the appropriate pod similar to manually
spawning a new process with 'cilium hubble port-forward'.

Signed-off-by: Alexandre Barone <abalexandrebarone@gmail.com>
Replace kubectl-based port-forwarding with a native
implementation inspired by the kubectl port-forward
cmd introduced by the PortForwarder helper type.

Signed-off-by: Alexandre Barone <abalexandrebarone@gmail.com>
auto-merge was automatically disabled October 31, 2024 13:10

Head branch was pushed to by a user without write access

@devodev devodev force-pushed the pr/devodev/hubble-auto-port-forward-native branch from 24a858c to 771193d Compare October 31, 2024 13:10
@devodev
Copy link
Contributor Author

devodev commented Oct 31, 2024

Fix PR has been merged and I force pushed a rebase from main.

@michi-covalent would you be kind enough to re-trigger tests when you can? 🥺 Thank you!

@michi-covalent
Copy link
Contributor

/test

@devodev
Copy link
Contributor Author

devodev commented Nov 1, 2024

That IPSec test is very flaky from looking at the action history. Hopefully a re-trigger will have it go through 🤞

@michi-covalent michi-covalent added this pull request to the merge queue Nov 1, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Nov 1, 2024
Merged via the queue into cilium:main with commit a6229a4 Nov 1, 2024
64 checks passed
michi-covalent added a commit to cilium/cilium-cli that referenced this pull request Nov 6, 2024
Update workflow files to accommodate cilium/cilium#35483.

Signed-off-by: Michi Mutsuzaki <michi@isovalent.com>
michi-covalent added a commit to cilium/cilium-cli that referenced this pull request Nov 6, 2024
Update workflow files to accommodate cilium/cilium#35483.

Signed-off-by: Michi Mutsuzaki <michi@isovalent.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cilium-cli This PR contains changes related with cilium-cli hubble-cli PRs or GitHub issues related with hubble-cli kind/community-contribution This was a contribution made by a community member. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/minor This PR changes functionality that users may find relevant to operating Cilium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CFP: Automatically port-forward Hubble CLI
9 participants