Skip to content

Commit 2f07bfd

Browse files
elasticdotventureselasticdotventures
committed
Refactor podman-env to use Docker API compatibility per review feedback
- Change podman-env to use Docker client against Podman's Docker-compatible socket - Remove SSH-based connectivity (Docker client doesn't support SSH keys) - Simplify podman-env to use standard Docker environment variables - Update tests to match new Docker API compatibility approach - Update documentation to clarify Docker client usage - Remove podman-specific test cases from docker-env_test.go This addresses the core review feedback about API compatibility issues and provides a cleaner separation between docker-env and podman-env.
1 parent 7723b9f commit 2f07bfd

File tree

4 files changed

+112
-222
lines changed

4 files changed

+112
-222
lines changed

‎cmd/minikube/cmd/docker-env_test.go

Lines changed: 0 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -429,16 +429,6 @@ SSH_AGENT_PID: "29228"
429429
}
430430
for _, tc := range tests {
431431
t.Run(tc.config.profile, func(t *testing.T) {
432-
origDetector := DockerBackendDetector
433-
defer func() { DockerBackendDetector = origDetector }()
434-
435-
// If this is the podman scenario, mock the backend detector
436-
if tc.config.profile == "podmandriver" {
437-
DockerBackendDetector = func(_ func(string, ...string) ([]byte, error)) (string, error) {
438-
return "podman", nil
439-
}
440-
tc.config.driver = "docker" // initial value, will be overridden by detector
441-
}
442432

443433
tc.config.EnvConfig.Shell = tc.shell
444434
// set global variable
@@ -465,36 +455,6 @@ SSH_AGENT_PID: "29228"
465455

466456
})
467457
}
468-
469-
// Add a Podman scenario test case
470-
t.Run("podmandriver", func(t *testing.T) {
471-
origDetector := DockerBackendDetector
472-
defer func() { DockerBackendDetector = origDetector }()
473-
DockerBackendDetector = func(_ func(string, ...string) ([]byte, error)) (string, error) {
474-
return "podman", nil
475-
}
476-
config := DockerEnvConfig{profile: "podmandriver", driver: "docker", hostIP: "127.0.0.1", port: 32842, certsDir: "/certs"}
477-
config.EnvConfig.Shell = "bash"
478-
var b []byte
479-
buf := bytes.NewBuffer(b)
480-
if err := dockerSetScript(config, buf); err != nil {
481-
t.Errorf("setScript(%+v) error: %v", config, err)
482-
}
483-
got := buf.String()
484-
// Podman should still output the same envs, but may add "existing" envs if present in the environment.
485-
// For this test, just check the main envs are present.
486-
wantSet := `export DOCKER_TLS_VERIFY="1"
487-
export DOCKER_HOST="tcp://127.0.0.1:32842"
488-
export DOCKER_CERT_PATH="/certs"
489-
export MINIKUBE_ACTIVE_DOCKERD="podmandriver"
490-
491-
# To point your shell to minikube's docker-daemon, run:
492-
# eval $(minikube -p podmandriver docker-env)
493-
`
494-
if diff := cmp.Diff(wantSet, got); diff != "" {
495-
t.Errorf("setScript(podmandriver) mismatch (-want +got):\n%s\n\nraw output:\n%s\nquoted: %q", diff, got, got)
496-
}
497-
})
498458
}
499459

500460
func TestValidDockerProxy(t *testing.T) {

‎cmd/minikube/cmd/podman-env.go

Lines changed: 72 additions & 158 deletions
Original file line numberDiff line numberDiff line change
@@ -21,85 +21,71 @@ import (
2121
"io"
2222
"os"
2323
"os/exec"
24-
"strings"
2524

26-
"github.com/docker/machine/libmachine/drivers"
27-
"github.com/docker/machine/libmachine/ssh"
2825
"github.com/spf13/cobra"
2926
"k8s.io/minikube/pkg/drivers/kic/oci"
3027
"k8s.io/minikube/pkg/minikube/command"
3128
"k8s.io/minikube/pkg/minikube/constants"
3229
"k8s.io/minikube/pkg/minikube/driver"
3330
"k8s.io/minikube/pkg/minikube/exit"
31+
"k8s.io/minikube/pkg/minikube/localpath"
3432
"k8s.io/minikube/pkg/minikube/mustload"
3533
"k8s.io/minikube/pkg/minikube/out"
3634
"k8s.io/minikube/pkg/minikube/reason"
3735
"k8s.io/minikube/pkg/minikube/shell"
3836
)
3937

40-
var podmanEnv1Tmpl = fmt.Sprintf(
41-
"{{ .Prefix }}%s{{ .Delimiter }}{{ .VarlinkBridge }}{{ .Suffix }}"+
42-
"{{ .Prefix }}%s{{ .Delimiter }}{{ .MinikubePodmanProfile }}{{ .Suffix }}"+
43-
"{{ .UsageHint }}",
44-
constants.PodmanVarlinkBridgeEnv,
45-
constants.MinikubeActivePodmanEnv)
46-
47-
var podmanEnv2Tmpl = fmt.Sprintf(
48-
"{{ .Prefix }}%s{{ .Delimiter }}{{ .ContainerHost }}{{ .Suffix }}"+
49-
"{{ if .ContainerSSHKey }}"+
50-
"{{ .Prefix }}%s{{ .Delimiter }}{{ .ContainerSSHKey}}{{ .Suffix }}"+
38+
var podmanEnvTmpl = fmt.Sprintf(
39+
"{{ .Prefix }}%s{{ .Delimiter }}{{ .DockerHost }}{{ .Suffix }}"+
40+
"{{ if .DockerTLSVerify }}"+
41+
"{{ .Prefix }}%s{{ .Delimiter }}{{ .DockerTLSVerify }}{{ .Suffix }}"+
42+
"{{ end }}"+
43+
"{{ if .DockerCertPath }}"+
44+
"{{ .Prefix }}%s{{ .Delimiter }}{{ .DockerCertPath }}{{ .Suffix }}"+
5145
"{{ end }}"+
52-
"{{ if .ExistingContainerHost }}"+
53-
"{{ .Prefix }}%s{{ .Delimiter }}{{ .ExistingContainerHost }}{{ .Suffix }}"+
46+
"{{ if .ExistingDockerHost }}"+
47+
"{{ .Prefix }}%s{{ .Delimiter }}{{ .ExistingDockerHost }}{{ .Suffix }}"+
5448
"{{ end }}"+
5549
"{{ .Prefix }}%s{{ .Delimiter }}{{ .MinikubePodmanProfile }}{{ .Suffix }}"+
5650
"{{ .UsageHint }}",
57-
constants.PodmanContainerHostEnv,
58-
constants.PodmanContainerSSHKeyEnv,
59-
constants.ExistingContainerHostEnv,
51+
constants.DockerHostEnv,
52+
constants.DockerTLSVerifyEnv,
53+
constants.DockerCertPathEnv,
54+
constants.ExistingDockerHostEnv,
6055
constants.MinikubeActivePodmanEnv)
6156

6257
// PodmanShellConfig represents the shell config for Podman
6358
type PodmanShellConfig struct {
6459
shell.Config
65-
VarlinkBridge string
66-
ContainerHost string
67-
ContainerSSHKey string
60+
DockerHost string
61+
DockerTLSVerify string
62+
DockerCertPath string
6863
MinikubePodmanProfile string
6964

70-
ExistingContainerHost string
65+
ExistingDockerHost string
7166
}
7267

7368
var podmanUnset bool
7469

7570
// podmanShellCfgSet generates context variables for "podman-env"
7671
func podmanShellCfgSet(ec PodmanEnvConfig, envMap map[string]string) *PodmanShellConfig {
7772
profile := ec.profile
78-
const usgPlz = "To point your shell to minikube's podman service, run:"
73+
const usgPlz = "To point your shell to minikube's podman docker-compatible service, run:"
7974
usgCmd := fmt.Sprintf("minikube -p %s podman-env", profile)
8075
s := &PodmanShellConfig{
8176
Config: *shell.CfgSet(ec.EnvConfig, usgPlz, usgCmd),
8277
}
83-
s.VarlinkBridge = envMap[constants.PodmanVarlinkBridgeEnv]
84-
s.ContainerHost = envMap[constants.PodmanContainerHostEnv]
85-
s.ContainerSSHKey = envMap[constants.PodmanContainerSSHKeyEnv]
78+
s.DockerHost = envMap[constants.DockerHostEnv]
79+
s.DockerTLSVerify = envMap[constants.DockerTLSVerifyEnv]
80+
s.DockerCertPath = envMap[constants.DockerCertPathEnv]
8681

87-
s.ExistingContainerHost = envMap[constants.ExistingContainerHostEnv]
82+
s.ExistingDockerHost = envMap[constants.ExistingDockerHostEnv]
8883

8984
s.MinikubePodmanProfile = envMap[constants.MinikubeActivePodmanEnv]
9085

9186
return s
9287
}
9388

94-
// isVarlinkAvailable checks if varlink command is available
95-
func isVarlinkAvailable(r command.Runner) bool {
96-
if _, err := r.RunCmd(exec.Command("which", "varlink")); err != nil {
97-
return false
98-
}
99-
100-
return true
101-
}
102-
10389
// isPodmanAvailable checks if podman command is available
10490
func isPodmanAvailable(r command.Runner) bool {
10591
if _, err := r.RunCmd(exec.Command("which", "podman")); err != nil {
@@ -109,35 +95,11 @@ func isPodmanAvailable(r command.Runner) bool {
10995
return true
11096
}
11197

112-
func createExternalSSHClient(d drivers.Driver) (*ssh.ExternalClient, error) {
113-
sshBinaryPath, err := exec.LookPath("ssh")
114-
if err != nil {
115-
return &ssh.ExternalClient{}, err
116-
}
117-
118-
addr, err := d.GetSSHHostname()
119-
if err != nil {
120-
return &ssh.ExternalClient{}, err
121-
}
122-
123-
port, err := d.GetSSHPort()
124-
if err != nil {
125-
return &ssh.ExternalClient{}, err
126-
}
127-
128-
auth := &ssh.Auth{}
129-
if d.GetSSHKeyPath() != "" {
130-
auth.Keys = []string{d.GetSSHKeyPath()}
131-
}
132-
133-
return ssh.NewExternalClient(sshBinaryPath, d.GetSSHUsername(), addr, port, auth)
134-
}
135-
13698
// podmanEnvCmd represents the podman-env command
13799
var podmanEnvCmd = &cobra.Command{
138100
Use: "podman-env",
139101
Short: "Configure environment to use minikube's Podman service",
140-
Long: `Sets up podman env variables; similar to '$(podman-machine env)'.`,
102+
Long: `Sets up Docker client env variables to use minikube's Podman Docker-compatible service.`,
141103
Run: func(_ *cobra.Command, _ []string) {
142104
sh := shell.EnvConfig{
143105
Shell: shell.ForceShell,
@@ -177,37 +139,38 @@ var podmanEnvCmd = &cobra.Command{
177139
exit.Message(reason.EnvPodmanUnavailable, `The podman service within '{{.cluster}}' is not active`, out.V{"cluster": cname})
178140
}
179141

180-
varlink := isVarlinkAvailable(r)
181-
182142
d := co.CP.Host.Driver
183-
client, err := createExternalSSHClient(d)
184-
if err != nil {
185-
exit.Error(reason.IfSSHClient, "Error getting ssh client", err)
186-
}
187-
188-
hostname, err := d.GetSSHHostname()
189-
if err != nil {
190-
exit.Error(reason.IfSSHClient, "Error getting ssh client", err)
191-
}
192-
193-
port, err := d.GetSSHPort()
194-
if err != nil {
195-
exit.Error(reason.IfSSHClient, "Error getting ssh client", err)
143+
hostIP := co.CP.IP.String()
144+
145+
// Use Docker API compatibility - podman supports Docker API on port 2376
146+
port := constants.DockerDaemonPort
147+
noProxy := false
148+
149+
// Check if we need to use SSH tunnel for remote access
150+
sshHost := false
151+
if driver.NeedsPortForward(driverName) {
152+
sshHost = true
153+
sshPort, err := d.GetSSHPort()
154+
if err != nil {
155+
exit.Error(reason.IfSSHClient, "Error getting ssh port", err)
156+
}
157+
hostIP = "127.0.0.1"
158+
_ = sshPort // We'll use SSH tunnel if needed
196159
}
197160

198161
ec := PodmanEnvConfig{
199162
EnvConfig: sh,
200163
profile: cname,
201164
driver: driverName,
202-
varlink: varlink,
203-
client: client,
204-
username: d.GetSSHUsername(),
205-
hostname: hostname,
165+
ssh: sshHost,
166+
hostIP: hostIP,
206167
port: port,
207-
keypath: d.GetSSHKeyPath(),
168+
certsDir: localpath.MakeMiniPath("certs"),
169+
noProxy: noProxy,
208170
}
209171

210172
if ec.Shell == "" {
173+
var err error
211174
ec.Shell, err = shell.Detect()
212175
if err != nil {
213176
exit.Error(reason.InternalShellDetect, "Error detecting shell", err)
@@ -225,22 +188,15 @@ type PodmanEnvConfig struct {
225188
shell.EnvConfig
226189
profile string
227190
driver string
228-
varlink bool
229-
client *ssh.ExternalClient
230-
username string
231-
hostname string
191+
ssh bool
192+
hostIP string
232193
port int
233-
keypath string
194+
certsDir string
195+
noProxy bool
234196
}
235197

236198
// podmanSetScript writes out a shell-compatible 'podman-env' script
237199
func podmanSetScript(ec PodmanEnvConfig, w io.Writer) error {
238-
var podmanEnvTmpl string
239-
if ec.varlink {
240-
podmanEnvTmpl = podmanEnv1Tmpl
241-
} else {
242-
podmanEnvTmpl = podmanEnv2Tmpl
243-
}
244200
envVars := podmanEnvVars(ec)
245201
return shell.SetScript(w, podmanEnvTmpl, podmanShellCfgSet(ec, envVars))
246202
}
@@ -251,85 +207,43 @@ func podmanUnsetScript(ec PodmanEnvConfig, w io.Writer) error {
251207
return shell.UnsetScript(ec.EnvConfig, w, vars)
252208
}
253209

254-
// podmanBridge returns the command to use in a var for accessing the podman varlink bridge over ssh
255-
func podmanBridge(client *ssh.ExternalClient) string {
256-
cmd := []string{client.BinaryPath}
257-
cmd = append(cmd, client.BaseArgs...)
258-
cmd = append(cmd, "--", "sudo", "varlink", "-A", `\'podman varlink \\\$VARLINK_ADDRESS\'`, "bridge")
259-
return strings.Join(cmd, " ")
260-
}
261-
262-
// podmanURL returns the url to use in a var for accessing the podman socket over ssh
263-
func podmanURL(username string, hostname string, port int) string {
264-
path := "/run/podman/podman.sock"
265-
return fmt.Sprintf("ssh://%s@%s:%d%s", username, hostname, port, path)
266-
}
267210

268-
// podmanEnvVars gets the necessary podman env variables to allow the use of minikube's podman service
211+
// podmanEnvVars gets the necessary Docker-compatible env variables for podman service
269212
func podmanEnvVars(ec PodmanEnvConfig) map[string]string {
270-
// podman v1
271-
env1 := map[string]string{
272-
constants.PodmanVarlinkBridgeEnv: podmanBridge(ec.client),
273-
}
274-
// podman v2
275-
env2 := map[string]string{
276-
constants.PodmanContainerHostEnv: podmanURL(ec.username, ec.hostname, ec.port),
277-
constants.PodmanContainerSSHKeyEnv: ec.keypath,
278-
}
279-
// common
280-
env0 := map[string]string{
281-
constants.MinikubeActivePodmanEnv: ec.profile,
282-
}
283-
284-
var env map[string]string
285-
if ec.varlink {
286-
env = env1
213+
var rt string
214+
if ec.ssh {
215+
rt = fmt.Sprintf("tcp://%s:%d", ec.hostIP, ec.port)
287216
} else {
288-
env = env2
217+
rt = fmt.Sprintf("tcp://%s:%d", ec.hostIP, ec.port)
289218
}
290-
for k, v := range env0 {
291-
env[k] = v
219+
220+
env := map[string]string{
221+
constants.DockerHostEnv: rt,
222+
constants.DockerTLSVerifyEnv: "1",
223+
constants.DockerCertPathEnv: ec.certsDir,
224+
constants.MinikubeActivePodmanEnv: ec.profile,
292225
}
226+
227+
// Save existing Docker env if not already using minikube
293228
if os.Getenv(constants.MinikubeActivePodmanEnv) == "" {
294-
e := constants.PodmanContainerHostEnv
295-
if v := oci.InitialEnv(e); v != "" {
296-
key := constants.ExistingContainerHostEnv
297-
env[key] = v
229+
for _, envVar := range constants.DockerDaemonEnvs {
230+
if v := oci.InitialEnv(envVar); v != "" {
231+
key := constants.MinikubeExistingPrefix + envVar
232+
env[key] = v
233+
}
298234
}
299235
}
300236
return env
301237
}
302238

303-
// podmanEnvNames gets the necessary podman env variables to reset after using minikube's podman service
239+
// podmanEnvNames gets the necessary Docker env variables to reset after using minikube's podman service
304240
func podmanEnvNames(ec PodmanEnvConfig) []string {
305-
// podman v1
306-
vars1 := []string{
307-
constants.PodmanVarlinkBridgeEnv,
308-
}
309-
// podman v2
310-
vars2 := []string{
311-
constants.PodmanContainerHostEnv,
312-
constants.PodmanContainerSSHKeyEnv,
313-
}
314-
// common
315-
vars0 := []string{
241+
vars := []string{
242+
constants.DockerHostEnv,
243+
constants.DockerTLSVerifyEnv,
244+
constants.DockerCertPathEnv,
316245
constants.MinikubeActivePodmanEnv,
317246
}
318-
319-
var vars []string
320-
if ec.client != nil || ec.hostname != "" {
321-
// getting ec.varlink needs a running machine
322-
if ec.varlink {
323-
vars = vars1
324-
} else {
325-
vars = vars2
326-
}
327-
} else {
328-
// just unset *all* of the variables instead
329-
vars = vars1
330-
vars = append(vars, vars2...)
331-
}
332-
vars = append(vars, vars0...)
333247
return vars
334248
}
335249

0 commit comments

Comments
 (0)