Skip to content

Commit c7e31a3

Browse files
kimdcottrellndeloof
authored andcommitted
Squashing feature branch commits in order to add signoff message.
- added clarity with error handling. added test to show issue. - in manual testing, this fixes the issue and allows watch to run after rebuild - added cleanup back in - fixed issue where watch extnet rebuild test would start all containers listed in the fixture Signed-off-by: kimdcottrell <[email protected]>
1 parent 704a9fd commit c7e31a3

File tree

4 files changed

+125
-2
lines changed

4 files changed

+125
-2
lines changed

pkg/compose/create.go

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1124,13 +1124,26 @@ func (s *composeService) resolveExternalNetwork(ctx context.Context, n *types.Ne
11241124
networks, err := s.apiClient().NetworkList(ctx, moby.NetworkListOptions{
11251125
Filters: filters.NewArgs(filters.Arg("name", n.Name)),
11261126
})
1127+
11271128
if err != nil {
11281129
return err
11291130
}
11301131

1132+
if len(networks) == 0 {
1133+
networks, err = s.apiClient().NetworkList(ctx, moby.NetworkListOptions{
1134+
Filters: filters.NewArgs(filters.Arg("id", n.Name)),
1135+
})
1136+
if err != nil {
1137+
return err
1138+
}
1139+
}
1140+
11311141
// NetworkList API doesn't return the exact name match, so we can retrieve more than one network with a request
11321142
networks = utils.Filter(networks, func(net moby.NetworkResource) bool {
1133-
return net.Name == n.Name
1143+
// later in this function, the name is changed the to ID.
1144+
// this function is called during the rebuild stage of `compose watch`.
1145+
// we still require just one network back, but we need to run the search on the ID
1146+
return net.Name == n.Name || net.ID == n.Name
11341147
})
11351148

11361149
switch len(networks) {

pkg/compose/watch.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -438,7 +438,7 @@ func (s *composeService) handleWatchBatch(ctx context.Context, project *types.Pr
438438
},
439439
})
440440
if err != nil {
441-
fmt.Fprintf(s.stderr(), "Application failed to start after update\n")
441+
fmt.Fprintf(s.stderr(), "Application failed to start after update. Error: %v\n", err)
442442
}
443443
return nil
444444
}
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
2+
services:
3+
ext-alpine:
4+
build:
5+
dockerfile_inline: |-
6+
FROM alpine
7+
init: true
8+
command: sleep infinity
9+
develop:
10+
watch:
11+
- action: rebuild
12+
path: .env
13+
networks:
14+
- external_network_test
15+
16+
networks:
17+
external_network_test:
18+
name: e2e-watch-external_network_test
19+
external: true

pkg/e2e/watch_test.go

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,97 @@ func TestWatch(t *testing.T) {
5555
})
5656
}
5757

58+
func TestRebuildOnDotEnvWithExternalNetwork(t *testing.T) {
59+
const projectName = "test_rebuild_on_dotenv_with_external_network"
60+
const svcName = "ext-alpine"
61+
containerName := strings.Join([]string{projectName, svcName, "1"}, "-")
62+
const networkName = "e2e-watch-external_network_test"
63+
const dotEnvFilepath = "./fixtures/watch/.env"
64+
65+
c := NewCLI(t, WithEnv(
66+
"COMPOSE_PROJECT_NAME="+projectName,
67+
"COMPOSE_FILE=./fixtures/watch/with-external-network.yaml",
68+
))
69+
70+
cleanup := func() {
71+
c.RunDockerComposeCmdNoCheck(t, "down", "--remove-orphans", "--volumes", "--rmi=local")
72+
c.RunDockerOrExitError(t, "network", "rm", networkName)
73+
os.Remove(dotEnvFilepath) //nolint:errcheck
74+
}
75+
cleanup()
76+
77+
t.Log("create network that is referenced by the container we're testing")
78+
c.RunDockerCmd(t, "network", "create", networkName)
79+
res := c.RunDockerCmd(t, "network", "ls")
80+
assert.Assert(t, !strings.Contains(res.Combined(), projectName), res.Combined())
81+
82+
t.Log("create a dotenv file that will be used to trigger the rebuild")
83+
os.WriteFile(dotEnvFilepath, []byte("HELLO=WORLD"), 0666)
84+
_, err := os.ReadFile(dotEnvFilepath)
85+
assert.NilError(t, err)
86+
87+
// TODO: refactor this duplicated code into frameworks? Maybe?
88+
t.Log("starting docker compose watch")
89+
cmd := c.NewDockerComposeCmd(t, "--verbose", "watch", svcName)
90+
// stream output since watch runs in the background
91+
cmd.Stdout = os.Stdout
92+
cmd.Stderr = os.Stderr
93+
r := icmd.StartCmd(cmd)
94+
require.NoError(t, r.Error)
95+
var testComplete atomic.Bool
96+
go func() {
97+
// if the process exits abnormally before the test is done, fail the test
98+
if err := r.Cmd.Wait(); err != nil && !t.Failed() && !testComplete.Load() {
99+
assert.Check(t, cmp.Nil(err))
100+
}
101+
}()
102+
103+
t.Log("wait for watch to start watching")
104+
c.WaitForCondition(t, func() (bool, string) {
105+
out := r.String()
106+
errors := r.String()
107+
return strings.Contains(out,
108+
"watching"), fmt.Sprintf("'watching' not found in : \n%s\nStderr: \n%s\n", out,
109+
errors)
110+
}, 30*time.Second, 1*time.Second)
111+
112+
n := c.RunDockerCmd(t, "network", "inspect", networkName, "-f", "{{ .Id }}")
113+
pn := c.RunDockerCmd(t, "inspect", containerName, "-f", "{{ .HostConfig.NetworkMode }}")
114+
assert.Equal(t, pn.Stdout(), n.Stdout())
115+
116+
t.Log("create a dotenv file that will be used to trigger the rebuild")
117+
os.WriteFile(dotEnvFilepath, []byte("HELLO=WORLD\nTEST=REBUILD"), 0666)
118+
_, err = os.ReadFile(dotEnvFilepath)
119+
assert.NilError(t, err)
120+
121+
// NOTE: are there any other ways to check if the container has been rebuilt?
122+
t.Log("check if the container has been rebuild")
123+
c.WaitForCondition(t, func() (bool, string) {
124+
out := r.String()
125+
if strings.Count(out, "batch complete: service["+svcName+"]") != 1 {
126+
return false, fmt.Sprintf("container %s was not rebuilt", containerName)
127+
}
128+
return true, fmt.Sprintf("container %s was rebuilt", containerName)
129+
}, 30*time.Second, 1*time.Second)
130+
131+
n2 := c.RunDockerCmd(t, "network", "inspect", networkName, "-f", "{{ .Id }}")
132+
pn2 := c.RunDockerCmd(t, "inspect", containerName, "-f", "{{ .HostConfig.NetworkMode }}")
133+
assert.Equal(t, pn2.Stdout(), n2.Stdout())
134+
135+
assert.Check(t, !strings.Contains(r.Combined(), "Application failed to start after update"))
136+
137+
t.Cleanup(cleanup)
138+
t.Cleanup(func() {
139+
// IMPORTANT: watch doesn't exit on its own, don't leak processes!
140+
if r.Cmd.Process != nil {
141+
t.Logf("Killing watch process: pid[%d]", r.Cmd.Process.Pid)
142+
_ = r.Cmd.Process.Kill()
143+
}
144+
})
145+
testComplete.Store(true)
146+
147+
}
148+
58149
// NOTE: these tests all share a single Compose file but are safe to run concurrently
59150
func doTest(t *testing.T, svcName string, tarSync bool) {
60151
tmpdir := t.TempDir()

0 commit comments

Comments
 (0)