feat: implement retina shell CLI command #962
Conversation
| timeout time.Duration | ||
| ) | ||
|
|
||
| const defaultRetinaShellImageRepo = "ghcr.io/microsoft/retina/retina-shell" |
There was a problem hiding this comment.
Few questions about this part:
- I'd prefer to use MCR instead of ghcr, since AKS clusters are required to have access to MCR. Everything else in this repo seems to be using ghcr, but is it possible to use MCR instead? (can make sure the image is replicated there before merging this).
- Defaulting to the Retina CLI version for the image tag. This works when the version is set in release builds, but will fail otherwise since we're defaulting to 0.0.5 here could we bump this later to a version that includes retina shell? (once it exists)
Lines 12 to 14 in 2b109a3
There was a problem hiding this comment.
You could make it a var, have it set by default to GHCR and then patch this var at link time with MCR if needed.
There was a problem hiding this comment.
yeah, that's an option. Do we do any patching like that today when building the retina kubectl plugin? (Ideally the official plugin release would use MCR so it will "just work" in any AKS cluster)
| shellCmd.Flags().BoolVarP(&allowHostFilesystemWrite, "allow-host-filesystem-write", "w", false, "Allow write access to the host filesystem. Implies --mount-host-filesystem. Applies only to nodes, not pods.") | ||
| shellCmd.Flags().BoolVar(&hostPID, "host-pid", false, "Set HostPID on the shell container. Applies only to nodes, not pods.") | ||
| shellCmd.Flags().StringSliceVarP(&capabilities, "capabilities", "c", []string{}, "Add capabilities to the shell container") | ||
| shellCmd.Flags().DurationVar(&timeout, "timeout", 30*time.Second, "The maximum time to wait for the shell container to start") |
There was a problem hiding this comment.
looking for feedback about how to structure these flags. This was the simplest way I could think of that supported all the use cases I wanted to support (see docs/06-Troubleshooting/shell.md)
| @@ -0,0 +1,91 @@ | |||
| package shell | |||
There was a problem hiding this comment.
Wasn't sure how to test the other parts of this, as it requires a k8s cluster and interactive shell / tty, so I added unit tests for just the manifest generation. Open to suggestions about how to test this better, maybe adding E2E in a followup PR.
| } | ||
|
|
||
| osLabel := node.Labels["kubernetes.io/os"] | ||
| if osLabel != "linux" { // Only Linux supported for now. |
There was a problem hiding this comment.
I tried supporting Windows, but got confused with how host process containers work, so I'm leaving this as a followup.
wedaly
force-pushed
the
retina-shell-feature-branch
branch
from
663ed7f
to
c20afa6
Compare
| return err | ||
| } | ||
|
|
||
| config := shell.Config{ |
There was a problem hiding this comment.
golang-ci lint is complaining that config isn't used, but it's definitely used on lines 106 and 108 below. Maybe it's getting confused because it's used in a closure?
There was a problem hiding this comment.
It's probably upset because it can't prove that the closure gets invoked, and it's trying to call r.Visit a liar. I'd just silence it since it's trying to be too smart and failing at it.
Signed-off-by: Will Daly <[email protected]>
Signed-off-by: Will Daly <[email protected]>
wedaly
force-pushed
the
retina-shell-feature-branch
branch
from
c20afa6
to
0c17452
Compare
Description
Implement
retina shellCLI command for adhoc network debugging of nodes and pods.Related Issue
#910
Checklist
git commit -S -s ...). See this documentation on signing commits.Screenshots (if applicable) or Testing Completed
Additional Notes
For testing, need to set
RETINA_SHELL_IMAGE_REPOandRETINA_SHELL_IMAGE_VERSIONuntil the retina-shell image is published.Please refer to the CONTRIBUTING.md file for more information on how to contribute to this project.