Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

registry: tweak how desired image is reconciled #345

Merged
merged 1 commit into from
Jun 18, 2024
Merged

registry: tweak how desired image is reconciled #345

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 23 additions & 6 deletions pkg/registry/registry.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,6 @@ func FillDefaults(registry *api.Registry) {
if registry.Name == "" {
registry.Name = "ctlptl-registry"
}

if registry.Image == "" {
registry.Image = DefaultRegistryImageRef
}
}

type socatController interface {
Expand Down Expand Up @@ -212,7 +208,11 @@ func (c *Controller) Apply(ctx context.Context, desired *api.Registry) (*api.Reg
// If the port has changed, let's delete the registry and recreate it.
needsDelete = true
}
if !imagesRefsEqual(existing.Status.Image, desired.Image) {
// If the desired image is different
// from the existing image, we need
// to delete the registry and recreate it.
if existing.Status.Image != "" && desired.Image != "" &&
!imagesRefsEqual(existing.Status.Image, desired.Image) {
needsDelete = true
}
if existing.Status.State != containerStateRunning {
Expand Down Expand Up @@ -284,13 +284,15 @@ func (c *Controller) Apply(ctx context.Context, desired *api.Registry) (*api.Reg
return nil, err
}

image := c.imageConfig(existing, desired)

err = dctr.Run(
ctx,
c.dockerCLI,
desired.Name,
&container.Config{
Hostname: desired.Name,
Image: desired.Image,
Image: image,
ExposedPorts: exposedPorts,
Labels: c.labelConfigs(existing, desired),
Env: desired.Env,
Expand Down Expand Up @@ -377,6 +379,21 @@ func (c *Controller) labelConfigs(existing *api.Registry, desired *api.Registry)
return newLabels
}

// Compute the image to ContainerCreate() call
func (c *Controller) imageConfig(existing *api.Registry, desired *api.Registry) string {
// Desired image takes precedence.
if desired.Image != "" {
return desired.Image
}

// Preserve existing image when possible.
if existing.Status.Image != "" {
return existing.Status.Image
}

return DefaultRegistryImageRef
}

func (c *Controller) maybeCreateForwarder(ctx context.Context, port int) error {
if docker.IsLocalHost(c.dockerCLI.Client().DaemonHost()) {
return nil
Expand Down
74 changes: 37 additions & 37 deletions pkg/registry/registry_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func kindRegistry() types.Container {
return types.Container{
ID: "a815c0ec15f1f7430bd402e3fffe65026dd692a1a99861a52b3e30ad6e253a08",
Names: []string{"/kind-registry"},
Image: "registry:2",
Image: DefaultRegistryImageRef,
ImageID: "sha256:2d4f4b5309b1e41b4f83ae59b44df6d673ef44433c734b14c1c103ebca82c116",
Command: "/entrypoint.sh /etc/docker/registry/config.yml",
Created: 1603483645,
Expand Down Expand Up @@ -56,7 +56,7 @@ func kindRegistryLoopback() types.Container {
return types.Container{
ID: "d62f2587ff7b03858f144d3cf83c789578a6d6403f8b82a459ab4e317917cd42",
Names: []string{"/kind-registry-loopback"},
Image: "registry:2",
Image: DefaultRegistryImageRef,
ImageID: "sha256:2d4f4b5309b1e41b4f83ae59b44df6d673ef44433c734b14c1c103ebca82c116",
Command: "/entrypoint.sh /etc/docker/registry/config.yml",
Created: 1603483646,
Expand Down Expand Up @@ -139,7 +139,7 @@ func TestListRegistries(t *testing.T) {
ContainerID: "a815c0ec15f1f7430bd402e3fffe65026dd692a1a99861a52b3e30ad6e253a08",
State: "running",
Labels: map[string]string{"dev.tilt.ctlptl.role": "registry"},
Image: "registry:2",
Image: DefaultRegistryImageRef,
Env: []string{"REGISTRY_STORAGE_DELETE_ENABLED=true", "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"},
},
}, list.Items[0])
Expand Down Expand Up @@ -174,7 +174,7 @@ func TestListRegistries(t *testing.T) {
Networks: []string{"bridge", "kind"},
ContainerID: "d62f2587ff7b03858f144d3cf83c789578a6d6403f8b82a459ab4e317917cd42",
State: "running",
Image: "registry:2",
Image: DefaultRegistryImageRef,
Env: []string{"REGISTRY_STORAGE_DELETE_ENABLED=true", "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"},
},
}, list.Items[2])
Expand Down Expand Up @@ -202,7 +202,7 @@ func TestGetRegistry(t *testing.T) {
ContainerID: "a815c0ec15f1f7430bd402e3fffe65026dd692a1a99861a52b3e30ad6e253a08",
State: "running",
Labels: map[string]string{"dev.tilt.ctlptl.role": "registry"},
Image: "registry:2",
Image: DefaultRegistryImageRef,
Env: []string{"REGISTRY_STORAGE_DELETE_ENABLED=true", "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"},
},
}, registry)
Expand All @@ -216,10 +216,6 @@ func TestApplyDeadRegistry(t *testing.T) {
deadRegistry.State = "dead"
f.docker.containers = []types.Container{deadRegistry}

f.docker.onCreate = func() {
f.docker.containers = []types.Container{kindRegistry()}
}

registry, err := f.c.Apply(context.Background(), &api.Registry{
TypeMeta: typeMeta,
Name: "kind-registry",
Expand All @@ -239,10 +235,6 @@ func TestApplyLabels(t *testing.T) {
// because it doesn't have the labels we want.
f.docker.containers = []types.Container{kindRegistry()}

f.docker.onCreate = func() {
f.docker.containers = []types.Container{kindRegistry()}
}

registry, err := f.c.Apply(context.Background(), &api.Registry{
TypeMeta: typeMeta,
Name: "kind-registry",
Expand All @@ -258,7 +250,7 @@ func TestApplyLabels(t *testing.T) {
"dev.tilt.ctlptl.role": "registry",
}, config.Labels)
assert.Equal(t, "kind-registry", config.Hostname)
assert.Equal(t, "docker.io/library/registry:2", config.Image)
assert.Equal(t, DefaultRegistryImageRef, config.Image)
assert.Equal(t, []string{"REGISTRY_STORAGE_DELETE_ENABLED=true"}, config.Env)
}
}
Expand All @@ -272,11 +264,6 @@ func TestPreservePort(t *testing.T) {
existingRegistry.Ports[0].PublicPort = 5010
f.docker.containers = []types.Container{existingRegistry}

// Running a command makes the registry come alive!
f.docker.onCreate = func() {
f.docker.containers = []types.Container{kindRegistry()}
}

registry, err := f.c.Apply(context.Background(), &api.Registry{
TypeMeta: typeMeta,
Name: "kind-registry",
Expand All @@ -289,7 +276,7 @@ func TestPreservePort(t *testing.T) {
if assert.NotNil(t, config) {
assert.Equal(t, map[string]string{"dev.tilt.ctlptl.role": "registry"}, config.Labels)
assert.Equal(t, "kind-registry", config.Hostname)
assert.Equal(t, "docker.io/library/registry:2", config.Image)
assert.Equal(t, DefaultRegistryImageRef, config.Image)
}
}

Expand All @@ -301,15 +288,11 @@ func TestCustomImage(t *testing.T) {
// because it doesn't have the image we want.
f.docker.containers = []types.Container{kindRegistry()}

f.docker.onCreate = func() {
f.docker.containers = []types.Container{kindRegistry()}
}

// ensure stable w/o image change
_, err := f.c.Apply(context.Background(), &api.Registry{
TypeMeta: typeMeta,
Name: "kind-registry",
Image: "registry:2",
Image: DefaultRegistryImageRef,
})
if assert.NoError(t, err) {
assert.Nil(t, f.docker.lastCreateConfig, "Registry should not have been re-created")
Expand All @@ -330,6 +313,26 @@ func TestCustomImage(t *testing.T) {
assert.Equal(t, "kind-registry", config.Hostname)
assert.Equal(t, "fake.tilt.dev/different-registry-image:latest", config.Image)
}

// Apply a config with new labels,
// ensure image is not changed.
registry, err = f.c.Apply(context.Background(), &api.Registry{
TypeMeta: typeMeta,
Name: "kind-registry",
Labels: map[string]string{"extra-label": "ctlptl"},
})
if assert.NoError(t, err) {
assert.Equal(t, "running", registry.Status.State)
}
config = f.docker.lastCreateConfig
if assert.NotNil(t, config) {
assert.Equal(t, map[string]string{
"dev.tilt.ctlptl.role": "registry",
"extra-label": "ctlptl",
}, config.Labels)
assert.Equal(t, "kind-registry", config.Hostname)
assert.Equal(t, "fake.tilt.dev/different-registry-image:latest", config.Image)
}
}

func TestCustomEnv(t *testing.T) {
Expand All @@ -340,15 +343,11 @@ func TestCustomEnv(t *testing.T) {
// because it doesn't have the image we want.
f.docker.containers = []types.Container{kindRegistry()}

f.docker.onCreate = func() {
f.docker.containers = []types.Container{kindRegistry()}
}

// ensure stable w/o image change
_, err := f.c.Apply(context.Background(), &api.Registry{
TypeMeta: typeMeta,
Name: "kind-registry",
Image: "registry:2",
Image: DefaultRegistryImageRef,
})
if assert.NoError(t, err) {
assert.Nil(t, f.docker.lastCreateConfig, "Registry should not have been re-created")
Expand All @@ -358,7 +357,7 @@ func TestCustomEnv(t *testing.T) {
registry, err := f.c.Apply(context.Background(), &api.Registry{
TypeMeta: typeMeta,
Name: "kind-registry",
Image: "registry:2",
Image: DefaultRegistryImageRef,
Env: []string{"REGISTRY_STORAGE_DELETE_ENABLED=false"},
})
if assert.NoError(t, err) {
Expand All @@ -368,7 +367,7 @@ func TestCustomEnv(t *testing.T) {
if assert.NotNil(t, config) {
assert.Equal(t, map[string]string{"dev.tilt.ctlptl.role": "registry"}, config.Labels)
assert.Equal(t, "kind-registry", config.Hostname)
assert.Equal(t, "registry:2", config.Image)
assert.Equal(t, DefaultRegistryImageRef, config.Image)
assert.Equal(t, []string{"REGISTRY_STORAGE_DELETE_ENABLED=false"}, config.Env)
}
}
Expand All @@ -390,7 +389,6 @@ type fakeDocker struct {
lastRemovedContainer string
lastCreateConfig *container.Config
lastCreateHostConfig *container.HostConfig
onCreate func()
}

type objectNotFoundError struct {
Expand Down Expand Up @@ -432,7 +430,7 @@ func (d *fakeDocker) ContainerInspect(ctx context.Context, containerID string) (
Cmd: []string{"/etc/docker/registry/config.yml"},
Healthcheck: (*container.HealthConfig)(nil),
ArgsEscaped: false,
Image: "docker.io/library/registry:2",
Image: DefaultRegistryImageRef,
Volumes: map[string]struct{}{"/var/lib/registry": struct{}{}},
WorkingDir: "",
Entrypoint: []string{"/entrypoint.sh"},
Expand Down Expand Up @@ -483,9 +481,11 @@ func (d *fakeDocker) ContainerCreate(ctx context.Context, config *container.Conf
containerName string) (container.CreateResponse, error) {
d.lastCreateConfig = config
d.lastCreateHostConfig = hostConfig
if d.onCreate != nil {
d.onCreate()
}

c := kindRegistry()
c.Image = config.Image
d.containers = []types.Container{c}

return container.CreateResponse{}, nil
}
func (d *fakeDocker) ContainerStart(ctx context.Context, containerID string,
Expand Down