Skip to content

Commit ddef948

Browse files
authored
[wanda] fix a bug of local image resolving (#88)
otherwise it is breaking backwards compatibility also added tests for the local resolving paths.
1 parent 572843b commit ddef948

15 files changed

+164
-56
lines changed

wanda/build_input_test.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -27,18 +27,18 @@ func TestBuildInputTags(t *testing.T) {
2727

2828
func TestBuildInputCore(t *testing.T) {
2929
ts := newTarStream()
30-
ts.addFile("Dockerfile", nil, "testdata/Dockerfile")
30+
ts.addFile("Dockerfile.hello", nil, "testdata/Dockerfile.hello")
3131

3232
in := newBuildInput(ts, []string{"MESSAGE=test=msg"})
3333
in.addTag("myimage")
3434

35-
core, err := in.makeCore("Dockerfile")
35+
core, err := in.makeCore("Dockerfile.hello")
3636
if err != nil {
3737
t.Fatalf("make build input core: %v", err)
3838
}
3939

40-
if core.Dockerfile != "Dockerfile" {
41-
t.Errorf("got %q, want Dockerfile", core.Dockerfile)
40+
if core.Dockerfile != "Dockerfile.hello" {
41+
t.Errorf("got %q, want Dockerfile.hello", core.Dockerfile)
4242
}
4343
if got := core.BuildArgs["MESSAGE"]; got != "test=msg" {
4444
t.Errorf("build args MESSAGE got %q, want `test=msg`", got)

wanda/docker_cmd.go

+6-10
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ func (c *dockerCmd) pull(src, asTag string) error {
6060
}
6161

6262
if src != asTag {
63-
if err := c.run("tag", src, asTag); err != nil {
63+
if err := c.tag(src, asTag); err != nil {
6464
return fmt.Errorf("tag %s %s: %w", src, asTag, err)
6565
}
6666
}
@@ -85,15 +85,11 @@ func (c *dockerCmd) build(in *buildInput, core *buildInputCore) error {
8585
if !ok {
8686
return fmt.Errorf("missing base image source for %q", from)
8787
}
88-
89-
if src.local != "" {
90-
if err := c.tag(src.local, src.name); err != nil {
91-
return fmt.Errorf("tag %s %s: %w", src.local, src.name, err)
92-
}
93-
} else {
94-
if err := c.pull(src.src, src.name); err != nil {
95-
return fmt.Errorf("pull %s(%s): %w", src.name, src.src, err)
96-
}
88+
if src.local != "" { // local image, already ready.
89+
continue
90+
}
91+
if err := c.pull(src.src, src.name); err != nil {
92+
return fmt.Errorf("pull %s(%s): %w", src.name, src.src, err)
9793
}
9894
}
9995
// TODO(aslonnie): maybe recheck all the IDs of the from images?

wanda/docker_cmd_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,15 @@ func TestDockerCmdBuild(t *testing.T) {
1313
cmd := newDockerCmd("") // uses real docker client
1414

1515
ts := newTarStream()
16-
ts.addFile("Dockerfile", nil, "testdata/Dockerfile")
16+
ts.addFile("Dockerfile.hello", nil, "testdata/Dockerfile.hello")
1717

1818
const tag = "cr.ray.io/rayproject/wanda-test"
1919

2020
buildArgs := []string{"MESSAGE=test mesasge"}
2121
input := newBuildInput(ts, buildArgs)
2222
input.addTag(tag)
2323

24-
core, err := input.makeCore("Dockerfile")
24+
core, err := input.makeCore("Dockerfile.hello")
2525
if err != nil {
2626
t.Fatalf("make build input core: %v", err)
2727
}

wanda/forge.go

+12
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,18 @@ func (f *Forge) resolveBases(froms []string) (map[string]*imageSource, error) {
9393
}
9494

9595
if namePrefix != "" && strings.HasPrefix(from, namePrefix) {
96+
if f.config.WorkRepo == "" {
97+
// Treat it as a local image.
98+
src, err := resolveLocalImage(from, from)
99+
if err != nil {
100+
return nil, fmt.Errorf(
101+
"resolve prefixed local image %s: %w", from, err,
102+
)
103+
}
104+
m[from] = src
105+
continue
106+
}
107+
96108
// An image in the work namespace. It is generated by a previous
97109
// job, and we need to pull it from the work repo.
98110
fromName := strings.TrimPrefix(from, f.config.NamePrefix)

wanda/forge_test.go

+119-35
Original file line numberDiff line numberDiff line change
@@ -11,17 +11,96 @@ import (
1111

1212
"github.com/google/go-containerregistry/pkg/name"
1313
"github.com/google/go-containerregistry/pkg/registry"
14+
cranev1 "github.com/google/go-containerregistry/pkg/v1"
1415
"github.com/google/go-containerregistry/pkg/v1/daemon"
1516
"github.com/google/go-containerregistry/pkg/v1/remote"
1617
)
1718

19+
func filesInLayer(layer cranev1.Layer) (map[string]string, error) {
20+
rc, err := layer.Uncompressed()
21+
if err != nil {
22+
return nil, fmt.Errorf("uncompress layer: %w", err)
23+
}
24+
defer rc.Close()
25+
26+
tr := tar.NewReader(rc)
27+
28+
files := make(map[string]string)
29+
30+
for {
31+
hdr, err := tr.Next()
32+
if err == io.EOF {
33+
break
34+
}
35+
if err != nil {
36+
return nil, fmt.Errorf("read tar header: %w", err)
37+
}
38+
39+
if hdr.FileInfo().IsDir() {
40+
continue
41+
}
42+
43+
content, err := io.ReadAll(tr)
44+
if err != nil {
45+
return nil, fmt.Errorf("read tar content: %w", err)
46+
}
47+
48+
files[hdr.Name] = string(content)
49+
}
50+
51+
return files, nil
52+
}
53+
54+
const worldDotTxt = "This is my world!"
55+
56+
func TestForgeLocal(t *testing.T) {
57+
config := &ForgeConfig{WorkDir: "testdata"}
58+
59+
if err := Build("testdata/localbase.wanda.yaml", config); err != nil {
60+
t.Fatalf("build base: %v", err)
61+
}
62+
63+
if err := Build("testdata/local.wanda.yaml", config); err != nil {
64+
t.Fatalf("build: %v", err)
65+
}
66+
67+
const resultRef = "cr.ray.io/rayproject/local"
68+
ref, err := name.ParseReference(resultRef)
69+
if err != nil {
70+
t.Fatalf("parse reference: %v", err)
71+
}
72+
73+
img, err := daemon.Image(ref)
74+
if err != nil {
75+
t.Fatalf("read image: %v", err)
76+
}
77+
78+
layers, err := img.Layers()
79+
if err != nil {
80+
t.Fatalf("read layers: %v", err)
81+
}
82+
83+
if len(layers) != 1 {
84+
t.Fatalf("got %d layers, want 2", len(layers))
85+
}
86+
87+
files, err := filesInLayer(layers[0])
88+
if err != nil {
89+
t.Fatalf("read layer: %v", err)
90+
}
91+
92+
if _, ok := files["opt/app/Dockerfile"]; !ok {
93+
t.Errorf("Dockerfile not in image")
94+
}
95+
}
96+
1897
func TestForge(t *testing.T) {
1998
config := &ForgeConfig{
2099
WorkDir: "testdata",
21100
NamePrefix: "cr.ray.io/rayproject/",
22101
}
23102

24-
if err := Build("testdata/hello.spec.yaml", config); err != nil {
103+
if err := Build("testdata/hello.wanda.yaml", config); err != nil {
25104
t.Fatalf("build: %v", err)
26105
}
27106

@@ -45,6 +124,39 @@ func TestForge(t *testing.T) {
45124
if len(layers) != 1 {
46125
t.Fatalf("got %d layers, want 1", len(layers))
47126
}
127+
128+
if err := Build("testdata/world.wanda.yaml", config); err != nil {
129+
t.Fatalf("build world: %v", err)
130+
}
131+
132+
world := "cr.ray.io/rayproject/world"
133+
ref2, err := name.ParseReference(world)
134+
if err != nil {
135+
t.Fatalf("parse world reference: %v", err)
136+
}
137+
138+
img2, err := daemon.Image(ref2)
139+
if err != nil {
140+
t.Fatalf("read world image: %v", err)
141+
}
142+
143+
layers2, err := img2.Layers()
144+
if err != nil {
145+
t.Fatalf("read world layers: %v", err)
146+
}
147+
148+
if len(layers2) != 2 {
149+
t.Fatalf("got %d world layers, want 1", len(layers2))
150+
}
151+
152+
files, err := filesInLayer(layers2[1])
153+
if err != nil {
154+
t.Fatalf("read world layer files: %v", err)
155+
}
156+
157+
if got := files["opt/app/world.txt"]; got != worldDotTxt {
158+
t.Errorf("world.txt in image, got %q, want %q", got, worldDotTxt)
159+
}
48160
}
49161

50162
func TestForgeWithWorkRepo(t *testing.T) {
@@ -67,11 +179,11 @@ func TestForgeWithWorkRepo(t *testing.T) {
67179
BuildID: "abc123",
68180
}
69181

70-
if err := Build("testdata/hello.spec.yaml", config); err != nil {
182+
if err := Build("testdata/hello.wanda.yaml", config); err != nil {
71183
t.Fatalf("build hello: %v", err)
72184
}
73185

74-
if err := Build("testdata/world.spec.yaml", config); err != nil {
186+
if err := Build("testdata/world.wanda.yaml", config); err != nil {
75187
t.Fatalf("build world: %v", err)
76188
}
77189

@@ -94,39 +206,11 @@ func TestForgeWithWorkRepo(t *testing.T) {
94206
if len(layers) != 2 {
95207
t.Fatalf("got %d layers, want 2", len(layers))
96208
}
97-
98-
layer, err := layers[1].Uncompressed()
209+
files, err := filesInLayer(layers[1])
99210
if err != nil {
100-
t.Fatalf("uncompress layer: %v", err)
211+
t.Fatalf("read layer: %v", err)
101212
}
102-
103-
tr := tar.NewReader(layer)
104-
105-
files := make(map[string]string)
106-
107-
for {
108-
hdr, err := tr.Next()
109-
if err == io.EOF {
110-
break
111-
}
112-
if err != nil {
113-
t.Fatalf("read tar header: %v", err)
114-
}
115-
116-
if hdr.FileInfo().IsDir() {
117-
continue
118-
}
119-
120-
content, err := io.ReadAll(tr)
121-
if err != nil {
122-
t.Fatalf("read tar content: %v", err)
123-
}
124-
125-
t.Log(hdr.Name)
126-
files[hdr.Name] = string(content)
127-
}
128-
129-
if got, want := files["opt/app/world.txt"], "This is my world!"; got != want {
130-
t.Errorf("world.txt in image, got %q, want %q", got, want)
213+
if got := files["opt/app/world.txt"]; got != worldDotTxt {
214+
t.Errorf("world.txt in image, got %q, want %q", got, worldDotTxt)
131215
}
132216
}

wanda/resolve_image.go

+3-2
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,9 @@ func resolveLocalImage(name, ref string) (*imageSource, error) {
2424
}
2525

2626
return &imageSource{
27-
name: name,
28-
id: id.String(),
27+
name: name,
28+
id: id.String(),
29+
local: ref,
2930
}, nil
3031
}
3132

wanda/resolve_image_test.go

+3
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,9 @@ func TestResolveLocalImage(t *testing.T) {
4848
if want := imageID.String(); want != src.id {
4949
t.Errorf("got image id %q, want %q", src.id, want)
5050
}
51+
if src.local != tagStr {
52+
t.Errorf("got image local %q, want %q", src.local, tagStr)
53+
}
5154

5255
dockerCmd := newDockerCmd("")
5356
if err := dockerCmd.run("image", "rm", tagStr); err != nil {

wanda/testdata/Dockerfile wanda/testdata/Dockerfile.hello

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,6 @@ FROM scratch
22

33
ARG MESSAGE=hello
44
ENV MESSAGE=${MESSAGE}
5-
COPY Dockerfile /opt/Dockerfile
5+
COPY Dockerfile.hello /opt/Dockerfile
66

77
CMD "echo ${MESSAGE}"

wanda/testdata/Dockerfile.local

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
FROM mybase
2+
3+
COPY Dockerfile.local /opt/app/Dockerfile

wanda/testdata/Dockerfile.localbase

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
FROM scratch

wanda/testdata/hello.spec.yaml

-2
This file was deleted.

wanda/testdata/hello.wanda.yaml

+2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
name: hello
2+
dockerfile: Dockerfile.hello

wanda/testdata/local.wanda.yaml

+4
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
name: local
2+
dockerfile: Dockerfile.local
3+
froms: [ "@mybase"]
4+
tags: [ "cr.ray.io/rayproject/local" ]

wanda/testdata/localbase.wanda.yaml

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
name: localbase
2+
dockerfile: Dockerfile.localbase
3+
tags: [ "mybase" ]
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
name: world
22
dockerfile: Dockerfile.world
3+
froms: [ "cr.ray.io/rayproject/hello" ]
34
srcs:
45
- world.txt

0 commit comments

Comments
 (0)