Skip to content

Commit be632ae

Browse files
notrepo05notrepo05
committed
wip: we no longer require session hijacking
we do not need to share our host ssh session
1 parent 3458d63 commit be632ae

2 files changed

Lines changed: 116 additions & 140 deletions

File tree

‎internal/test/Dockerfile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
# syntax=docker/dockerfile:1.10
22

33
FROM golang AS go-image
4-
FROM pivotalcfreleng/kiln:v0.108.0 AS kiln
4+
FROM docker.io/pivotalcfreleng/kiln:v0.110.0-rc2 AS kiln
55

66
FROM ruby:3.4.8 AS builder
77
RUN mkdir -p -m 0700 ~/.ssh && ssh-keyscan github.com >> ~/.ssh/known_hosts

‎internal/test/container.go‎

Lines changed: 115 additions & 139 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,13 @@ import (
1616
"path/filepath"
1717
"strings"
1818

19+
cerrdefs "github.com/containerd/errdefs"
1920
"github.com/docker/docker/api/types"
2021
"github.com/docker/docker/api/types/build"
2122
"github.com/docker/docker/api/types/container"
2223
"github.com/docker/docker/api/types/mount"
2324
"github.com/docker/docker/api/types/network"
2425
"github.com/docker/docker/client"
25-
"github.com/moby/buildkit/session"
2626
specV1 "github.com/opencontainers/image-spec/specs-go/v1"
2727
"golang.org/x/sync/errgroup"
2828
)
@@ -42,7 +42,7 @@ func Run(ctx context.Context, w io.Writer, configuration Configuration) error {
4242
return err
4343
}
4444

45-
return configureSession(ctx, logger, configuration, dockerDaemon, runTestWithSession(ctx, logger, w, dockerDaemon, configuration))
45+
return runTest(ctx, logger, w, dockerDaemon, configuration)
4646
}
4747

4848
type Configuration struct {
@@ -97,164 +97,140 @@ type mobyClient interface {
9797
ContainerStop(ctx context.Context, containerID string, options container.StopOptions) error
9898
}
9999

100-
func runTestWithSession(ctx context.Context, logger *log.Logger, w io.Writer, dockerDaemon mobyClient, configuration Configuration) func(sessionID string) error {
101-
return func(sessionID string) error {
102-
commands, err := configuration.commands()
103-
if err != nil {
104-
return err
105-
}
100+
func runTest(ctx context.Context, logger *log.Logger, w io.Writer, dockerDaemon mobyClient, configuration Configuration) error {
101+
logger.Printf("pinging docker daemon")
102+
_, err := dockerDaemon.Ping(ctx)
103+
if err != nil {
104+
return fmt.Errorf("failed to connect to Docker daemon: %w", err)
105+
}
106106

107-
var dockerfileTarball bytes.Buffer
108-
if err := createDockerfileTarball(tar.NewWriter(&dockerfileTarball), dockerfile); err != nil {
109-
return err
110-
}
107+
commands, err := configuration.commands()
108+
if err != nil {
109+
return err
110+
}
111111

112-
envMap, err := decodeEnvironment(configuration.Environment)
113-
if err != nil {
114-
return fmt.Errorf("failed to parse environment: %w", err)
115-
}
112+
var dockerfileTarball bytes.Buffer
113+
if err := createDockerfileTarball(tar.NewWriter(&dockerfileTarball), dockerfile); err != nil {
114+
return err
115+
}
116116

117-
artifactoryUsername := envMap["ARTIFACTORY_USERNAME"]
118-
artifactoryPassword := envMap["ARTIFACTORY_PASSWORD"]
119-
120-
logger.Println("creating test image")
121-
resp, err := dockerDaemon.ImageBuild(ctx, &dockerfileTarball, build.ImageBuildOptions{
122-
Tags: []string{"kiln_test_dependencies:vmware"},
123-
Version: build.BuilderBuildKit,
124-
SessionID: sessionID,
125-
BuildArgs: map[string]*string{
126-
"ARTIFACTORY_USERNAME": &artifactoryUsername,
127-
"ARTIFACTORY_PASSWORD": &artifactoryPassword,
128-
},
129-
})
117+
envMap, err := decodeEnvironment(configuration.Environment)
118+
if err != nil {
119+
return fmt.Errorf("failed to parse environment: %w", err)
120+
}
130121

131-
if err != nil {
132-
return fmt.Errorf("failed to build image: %w", err)
133-
}
122+
artifactoryUsername := envMap["ARTIFACTORY_USERNAME"]
123+
artifactoryPassword := envMap["ARTIFACTORY_PASSWORD"]
134124

135-
logger.Println("reading image build response")
136-
_, err = io.ReadAll(resp.Body)
137-
if err != nil {
138-
return fmt.Errorf("failed to read image build response: %w", err)
139-
}
125+
logger.Println("creating test image")
126+
resp, err := dockerDaemon.ImageBuild(ctx, &dockerfileTarball, build.ImageBuildOptions{
127+
Tags: []string{"kiln_test_dependencies:vmware"},
128+
Version: build.BuilderBuildKit,
129+
BuildArgs: map[string]*string{
130+
"ARTIFACTORY_USERNAME": &artifactoryUsername,
131+
"ARTIFACTORY_PASSWORD": &artifactoryPassword,
132+
},
133+
})
140134

141-
parentDir := path.Dir(configuration.AbsoluteTileDirectory)
142-
tileDir := path.Base(configuration.AbsoluteTileDirectory)
143-
144-
dockerCmd := strings.Join(commands, " && ")
145-
146-
envVars := getTileTestEnvVars(configuration.AbsoluteTileDirectory, tileDir, envMap)
147-
logger.Println("creating test container")
148-
testContainer, err := dockerDaemon.ContainerCreate(ctx, &container.Config{
149-
Image: "kiln_test_dependencies:vmware",
150-
Cmd: []string{"/bin/bash", "-c", dockerCmd},
151-
Env: encodeEnvironment(envVars),
152-
Tty: true,
153-
}, &container.HostConfig{
154-
LogConfig: container.LogConfig{
155-
Config: map[string]string{
156-
"mode": string(container.LogModeNonBlock),
157-
},
135+
if err != nil {
136+
return fmt.Errorf("failed to build image: %w", err)
137+
}
138+
139+
logger.Println("reading image build response")
140+
_, err = io.ReadAll(resp.Body)
141+
if err != nil {
142+
return fmt.Errorf("failed to read image build response: %w", err)
143+
}
144+
145+
parentDir := path.Dir(configuration.AbsoluteTileDirectory)
146+
tileDir := path.Base(configuration.AbsoluteTileDirectory)
147+
148+
dockerCmd := strings.Join(commands, " && ")
149+
150+
envVars := getTileTestEnvVars(configuration.AbsoluteTileDirectory, tileDir, envMap)
151+
logger.Println("creating test container")
152+
testContainer, err := dockerDaemon.ContainerCreate(ctx, &container.Config{
153+
Image: "kiln_test_dependencies:vmware",
154+
Cmd: []string{"/bin/bash", "-c", dockerCmd},
155+
Env: encodeEnvironment(envVars),
156+
Tty: true,
157+
}, &container.HostConfig{
158+
LogConfig: container.LogConfig{
159+
Config: map[string]string{
160+
"mode": string(container.LogModeNonBlock),
158161
},
159-
Mounts: []mount.Mount{
160-
{
161-
Type: mount.TypeBind,
162-
Source: parentDir,
163-
Target: "/tas",
164-
},
162+
},
163+
Mounts: []mount.Mount{
164+
{
165+
Type: mount.TypeBind,
166+
Source: parentDir,
167+
Target: "/tas",
165168
},
166-
AutoRemove: true,
167-
}, nil, nil, "")
168-
if err != nil {
169-
return fmt.Errorf("failed to create container: %w", err)
170-
}
171-
logger.Printf("created test container with id %s", testContainer.ID)
172-
173-
errG := errgroup.Group{}
174-
175-
sigInt := make(chan os.Signal, 1)
176-
signal.Notify(sigInt, os.Interrupt)
177-
errG.Go(func() error {
178-
<-sigInt
179-
err := dockerDaemon.ContainerStop(ctx, testContainer.ID, container.StopOptions{
180-
Signal: "SIGKILL",
181-
})
182-
if err != nil {
183-
return fmt.Errorf("failed to stop container: %w", err)
184-
}
185-
return nil
186-
})
169+
},
170+
AutoRemove: true,
171+
}, nil, nil, "")
172+
if err != nil {
173+
return fmt.Errorf("failed to create container: %w", err)
174+
}
175+
logger.Printf("created test container with id %s", testContainer.ID)
187176

188-
if err := dockerDaemon.ContainerStart(ctx, testContainer.ID, container.StartOptions{}); err != nil {
189-
return fmt.Errorf("failed to start test container: %w", err)
190-
}
177+
errG := errgroup.Group{}
191178

192-
out, err := dockerDaemon.ContainerLogs(ctx, testContainer.ID, container.LogsOptions{ShowStdout: true, ShowStderr: true, Follow: true})
179+
sigInt := make(chan os.Signal, 1)
180+
signal.Notify(sigInt, os.Interrupt)
181+
errG.Go(func() error {
182+
<-sigInt
183+
err := dockerDaemon.ContainerStop(ctx, testContainer.ID, container.StopOptions{
184+
Signal: "SIGKILL",
185+
})
193186
if err != nil {
194-
return fmt.Errorf("container log request failure: %w", err)
195-
}
196-
if _, err := io.Copy(w, out); err != nil {
197-
return err
198-
}
199-
200-
// Although the fan-in loop pattern seems like the right solution here, ContainerWait
201-
// does not properly close channels, so it won't work.
202-
var resultErr error
203-
statusCh, containerWaitError := dockerDaemon.ContainerWait(ctx, testContainer.ID, container.WaitConditionNotRunning)
204-
select {
205-
case err := <-containerWaitError:
206-
resultErr = err
207-
case status := <-statusCh:
208-
if status.StatusCode != 0 {
209-
if status.Error != nil {
210-
resultErr = fmt.Errorf("test failed with exit code %d: %s", status.StatusCode, status.Error.Message)
211-
} else {
212-
resultErr = fmt.Errorf("test failed with exit code %d", status.StatusCode)
213-
}
187+
fmt.Printf("%+v\n", err)
188+
fmt.Println(reflect.ValueOf(err).Type())
189+
if cerrdefs.IsNotFound(err) {
190+
return nil
191+
}
192+
if strings.Contains(err.Error(), "no such container") {
193+
return nil
214194
}
195+
return fmt.Errorf("failed to stop container: %w. %+v. %s", err, err, reflect.ValueOf(err).Type())
215196
}
216-
signal.Stop(sigInt)
217-
close(sigInt)
197+
return nil
198+
})
218199

219-
return errors.Join(resultErr, errG.Wait())
200+
if err := dockerDaemon.ContainerStart(ctx, testContainer.ID, container.StartOptions{}); err != nil {
201+
return fmt.Errorf("failed to start test container: %w", err)
220202
}
221-
}
222203

223-
// configureSession is the part of the code that sets up socket connections and interacts with the daemon
224-
// testing it is non-trivial, so I isolated it. Testing it properly would require a daemon connection.
225-
func configureSession(ctx context.Context, logger *log.Logger, configuration Configuration, dockerDaemon mobyClient, function func(sessionID string) error) error {
226-
logger.Printf("pinging docker daemon")
227-
_, err := dockerDaemon.Ping(ctx)
204+
out, err := dockerDaemon.ContainerLogs(ctx, testContainer.ID, container.LogsOptions{ShowStdout: true, ShowStderr: true, Follow: true})
228205
if err != nil {
229-
return fmt.Errorf("failed to connect to Docker daemon: %w", err)
206+
return fmt.Errorf("container log request failure: %w", err)
230207
}
231-
232-
s, err := session.NewSession(ctx, "waypoint")
233-
if err != nil {
234-
return fmt.Errorf("failed to create docker daemon session: %w", err)
208+
if _, err := io.Copy(w, out); err != nil {
209+
return err
235210
}
236-
defer closeAndIgnoreError(s)
237-
238-
runErrC := make(chan error)
239-
go func() {
240-
defer close(runErrC)
241-
runErrC <- s.Run(ctx, func(ctx context.Context, proto string, meta map[string][]string) (net.Conn, error) {
242-
conn, err := dockerDaemon.DialHijack(ctx, "/session", proto, meta)
243-
if err != nil {
244-
return nil, fmt.Errorf("session hijack error: %w", err)
245-
}
246-
return conn, nil
247-
})
248-
}()
249-
250-
logger.Println("completed session setup")
251211

252-
err = function(s.ID())
253-
_ = s.Close()
254-
for e := range runErrC {
255-
err = errors.Join(err, e)
212+
//Although the fan-in loop pattern seems like the right solution here, ContainerWait
213+
//does not properly close channels, so it won't work.
214+
var resultErr error
215+
statusCh, containerWaitError := dockerDaemon.ContainerWait(ctx, testContainer.ID, container.WaitConditionNotRunning)
216+
select {
217+
case err := <-containerWaitError:
218+
if !cerrdefs.IsNotFound(err) {
219+
resultErr = nil
220+
}
221+
case status := <-statusCh:
222+
if status.StatusCode != 0 {
223+
if status.Error != nil {
224+
resultErr = fmt.Errorf("test failed with exit code %d: %s", status.StatusCode, status.Error.Message)
225+
} else {
226+
resultErr = fmt.Errorf("test failed with exit code %d", status.StatusCode)
227+
}
228+
}
256229
}
257-
return err
230+
signal.Stop(sigInt)
231+
close(sigInt)
232+
233+
return errors.Join(resultErr, errG.Wait())
258234
}
259235

260236
type environmentVars = map[string]string

0 commit comments

Comments
 (0)