Skip to content

Commit

Permalink
tiltfile: support multiple images with the same name but different tag (
Browse files Browse the repository at this point in the history
#1297)

Fixes #1259
  • Loading branch information
nicks authored Mar 12, 2019
1 parent 9ef6497 commit 30ef55a
Show file tree
Hide file tree
Showing 12 changed files with 195 additions and 113 deletions.
51 changes: 40 additions & 11 deletions internal/container/selector.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,35 @@
package container

import "github.com/docker/distribution/reference"
import (
"github.com/docker/distribution/reference"
)

type MatchType int

const (
matchName MatchType = iota
matchExact
)

type RefSelector struct {
ref reference.Named
ref reference.Named
matchType MatchType
}

func NameSelector(ref reference.Named) RefSelector {
return NewRefSelector(reference.TrimNamed(ref))
}

func NewRefSelector(ref reference.Named) RefSelector {
return RefSelector{ref: ref}
matchType := matchName
_, hasTag := ref.(reference.NamedTagged)
if hasTag {
matchType = matchExact
}
return RefSelector{
ref: ref,
matchType: matchType,
}
}

func MustParseSelector(s string) RefSelector {
Expand All @@ -18,31 +40,38 @@ func MustParseTaggedSelector(s string) RefSelector {
return NewRefSelector(MustParseNamedTagged(s))
}

func (s RefSelector) RefsEqual(other RefSelector) bool {
return s.ref.String() == other.ref.String()
}

func (s RefSelector) WithExactMatch() RefSelector {
s.matchType = matchExact
return s
}

func (s RefSelector) Matches(toMatch reference.Named) bool {
if s.ref == nil {
return false
}
return toMatch.Name() == s.ref.Name()

if s.matchType == matchName {
return toMatch.Name() == s.ref.Name()
}
return toMatch.String() == s.ref.String()
}

func (s RefSelector) Empty() bool {
return s.ref == nil
}

func (s RefSelector) Name() string {
func (s RefSelector) RefName() string {
return s.ref.Name()
}

func (s RefSelector) AsNamedOnly() reference.Named {
return reference.TrimNamed(s.ref)
}

// TODO(nick): This method is only provided for legacy compatibility.
// All uses of this really should not be using the full ref.
func (s RefSelector) AsRef() reference.Named {
return s.ref
}

func (s RefSelector) String() string {
if s.ref == nil {
return ""
Expand Down
16 changes: 8 additions & 8 deletions internal/dockerfile/inject_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ FROM gcr.io/windmill/foo
ADD . .
`)
ref := container.MustParseNamedTagged("gcr.io/windmill/foo:deadbeef")
newDf, modified, err := InjectImageDigest(df, container.NewRefSelector(ref), ref)
newDf, modified, err := InjectImageDigest(df, container.NameSelector(ref), ref)
if assert.NoError(t, err) {
assert.True(t, modified)
assert.Equal(t, `
Expand All @@ -29,7 +29,7 @@ FROM gcr.io/windmill/foo:v1
ADD . .
`)
ref := container.MustParseNamedTagged("gcr.io/windmill/foo:deadbeef")
newDf, modified, err := InjectImageDigest(df, container.NewRefSelector(ref), ref)
newDf, modified, err := InjectImageDigest(df, container.NameSelector(ref), ref)
if assert.NoError(t, err) {
assert.True(t, modified)
assert.Equal(t, `
Expand All @@ -45,7 +45,7 @@ FROM gcr.io/windmill/bar:v1
ADD . .
`)
ref := container.MustParseNamedTagged("gcr.io/windmill/foo:deadbeef")
newDf, modified, err := InjectImageDigest(df, container.NewRefSelector(ref), ref)
newDf, modified, err := InjectImageDigest(df, container.NameSelector(ref), ref)
if assert.NoError(t, err) {
assert.False(t, modified)
assert.Equal(t, df, newDf)
Expand All @@ -59,7 +59,7 @@ COPY --from=gcr.io/windmill/foo /src/package.json /src/package.json
ADD . .
`)
ref := container.MustParseNamedTagged("gcr.io/windmill/foo:deadbeef")
newDf, modified, err := InjectImageDigest(df, container.NewRefSelector(ref), ref)
newDf, modified, err := InjectImageDigest(df, container.NameSelector(ref), ref)
if assert.NoError(t, err) {
assert.True(t, modified)
assert.Equal(t, `
Expand All @@ -77,7 +77,7 @@ COPY --from=gcr.io/windmill/foo:bar /src/package.json /src/package.json
ADD . .
`)
ref := container.MustParseNamedTagged("gcr.io/windmill/foo:deadbeef")
newDf, modified, err := InjectImageDigest(df, container.NewRefSelector(ref), ref)
newDf, modified, err := InjectImageDigest(df, container.NameSelector(ref), ref)
if assert.NoError(t, err) {
assert.True(t, modified)
assert.Equal(t, `
Expand All @@ -95,7 +95,7 @@ COPY --from=vandelay/common /usr/src/common/package.json /usr/src/common/yarn.lo
ADD . .
`)
ref := container.MustParseNamedTagged("vandelay/common:deadbeef")
newDf, modified, err := InjectImageDigest(df, container.NewRefSelector(ref), ref)
newDf, modified, err := InjectImageDigest(df, container.NameSelector(ref), ref)
if assert.NoError(t, err) {
assert.True(t, modified)
assert.Equal(t, `
Expand All @@ -118,7 +118,7 @@ ADD . .
t.Fatal(err)
}

modified, err := ast.InjectImageDigest(container.NewRefSelector(ref), ref)
modified, err := ast.InjectImageDigest(container.NameSelector(ref), ref)
if assert.NoError(t, err) {
assert.True(t, modified)

Expand All @@ -133,7 +133,7 @@ ADD . .
`, string(newDf))
}

modified, err = ast.InjectImageDigest(container.NewRefSelector(ref), ref)
modified, err = ast.InjectImageDigest(container.NameSelector(ref), ref)
if assert.NoError(t, err) {
assert.True(t, modified)

Expand Down
4 changes: 3 additions & 1 deletion internal/engine/image_build_and_deployer.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,8 +196,10 @@ func (ibd *ImageBuildAndDeployer) deploy(ctx context.Context, st store.RStore, p
injectedDepIDs[depID] = true

if ibd.injectSynclet && needsSynclet {
injectedRefSelector := container.NewRefSelector(ref).WithExactMatch()

var sidecarInjected bool
e, sidecarInjected, err = sidecar.InjectSyncletSidecar(e, selector)
e, sidecarInjected, err = sidecar.InjectSyncletSidecar(e, injectedRefSelector)
if err != nil {
return err
}
Expand Down
12 changes: 6 additions & 6 deletions internal/k8s/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -231,31 +231,31 @@ func (e K8sEntity) FindImages(imageJSONPaths []JSONPath) ([]reference.Named, err
return result, nil
}

func PodContainsRef(pod v1.PodSpec, ref reference.Named) (bool, error) {
cRef, err := FindImageRefMatching(pod, ref)
func PodContainsRef(pod v1.PodSpec, selector container.RefSelector) (bool, error) {
cRef, err := FindImageRefMatching(pod, selector)
if err != nil {
return false, err
}

return cRef != nil, nil
}

func FindImageRefMatching(pod v1.PodSpec, ref reference.Named) (reference.Named, error) {
func FindImageRefMatching(pod v1.PodSpec, selector container.RefSelector) (reference.Named, error) {
for _, c := range pod.Containers {
cRef, err := container.ParseNamed(c.Image)
if err != nil {
return nil, errors.Wrap(err, "FindImageRefMatching")
}

if cRef.Name() == ref.Name() {
if selector.Matches(cRef) {
return cRef, nil
}
}
return nil, nil
}

func FindImageNamedTaggedMatching(pod v1.PodSpec, ref reference.Named) (reference.NamedTagged, error) {
cRef, err := FindImageRefMatching(pod, ref)
func FindImageNamedTaggedMatching(pod v1.PodSpec, selector container.RefSelector) (reference.NamedTagged, error) {
cRef, err := FindImageRefMatching(pod, selector)
if err != nil {
return nil, err
}
Expand Down
8 changes: 4 additions & 4 deletions internal/k8s/image_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
"github.com/opencontainers/go-digest"
"github.com/stretchr/testify/assert"
appsv1 "k8s.io/api/apps/v1"
"k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"

"github.com/windmilleng/tilt/internal/container"
"github.com/windmilleng/tilt/internal/k8s/testyaml"
Expand Down Expand Up @@ -164,7 +164,7 @@ func TestInjectDigestBlorgBackendYAML(t *testing.T) {
entity := entities[1]
name := "gcr.io/blorg-dev/blorg-backend"
namedTagged, _ := reference.ParseNamed(fmt.Sprintf("%s:wm-tilt", name))
newEntity, replaced, err := InjectImageDigest(entity, container.NewRefSelector(namedTagged), namedTagged, v1.PullNever)
newEntity, replaced, err := InjectImageDigest(entity, container.NameSelector(namedTagged), namedTagged, v1.PullNever)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -204,7 +204,7 @@ func InjectImageDigestWithStrings(entity K8sEntity, original string, newDigest s
return K8sEntity{}, false, err
}

return InjectImageDigest(entity, container.NewRefSelector(originalRef), canonicalRef, policy)
return InjectImageDigest(entity, container.NameSelector(originalRef), canonicalRef, policy)
}

func TestInjectSyncletImage(t *testing.T) {
Expand All @@ -217,7 +217,7 @@ func TestInjectSyncletImage(t *testing.T) {
entity := entities[0]
name := "gcr.io/windmill-public-containers/synclet"
namedTagged, _ := container.ParseNamedTagged(fmt.Sprintf("%s:tilt-deadbeef", name))
newEntity, replaced, err := InjectImageDigest(entity, container.NewRefSelector(namedTagged), namedTagged, v1.PullNever)
newEntity, replaced, err := InjectImageDigest(entity, container.NameSelector(namedTagged), namedTagged, v1.PullNever)
if err != nil {
t.Fatal(err)
} else if !replaced {
Expand Down
3 changes: 3 additions & 0 deletions internal/model/manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"k8s.io/apimachinery/pkg/labels"

"github.com/docker/distribution/reference"
"github.com/windmilleng/tilt/internal/container"
"github.com/windmilleng/tilt/internal/sliceutils"

"github.com/google/go-cmp/cmp"
Expand Down Expand Up @@ -302,6 +303,7 @@ var imageTargetAllowUnexported = cmp.AllowUnexported(ImageTarget{})
var dcTargetAllowUnexported = cmp.AllowUnexported(DockerComposeTarget{})
var labelRequirementAllowUnexported = cmp.AllowUnexported(labels.Requirement{})
var k8sTargetAllowUnexported = cmp.AllowUnexported(K8sTarget{})
var selectorAllowUnexported = cmp.AllowUnexported(container.RefSelector{})

var dockerRefEqual = cmp.Comparer(func(a, b reference.Named) bool {
aNil := a == nil
Expand All @@ -324,5 +326,6 @@ func DeepEqual(x, y interface{}) bool {
dcTargetAllowUnexported,
labelRequirementAllowUnexported,
k8sTargetAllowUnexported,
selectorAllowUnexported,
dockerRefEqual)
}
7 changes: 4 additions & 3 deletions internal/synclet/sidecar/inject.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
package sidecar

import (
"github.com/docker/distribution/reference"
"github.com/windmilleng/tilt/internal/container"
"github.com/windmilleng/tilt/internal/k8s"
)

func InjectSyncletSidecar(entity k8s.K8sEntity, matchRef reference.Named) (k8s.K8sEntity, bool, error) {
// Inject the synclet into any Pod
func InjectSyncletSidecar(entity k8s.K8sEntity, selector container.RefSelector) (k8s.K8sEntity, bool, error) {
entity = entity.DeepCopy()

pods, err := k8s.ExtractPods(&entity)
Expand All @@ -15,7 +16,7 @@ func InjectSyncletSidecar(entity k8s.K8sEntity, matchRef reference.Named) (k8s.K

replaced := false
for _, pod := range pods {
ok, err := k8s.PodContainsRef(*pod, matchRef)
ok, err := k8s.PodContainsRef(*pod, selector)
if err != nil {
return k8s.K8sEntity{}, false, err
}
Expand Down
6 changes: 3 additions & 3 deletions internal/synclet/sidecar/sidecar_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ import (
"strings"
"testing"

"github.com/docker/distribution/reference"
"github.com/stretchr/testify/assert"
"github.com/windmilleng/tilt/internal/container"
"github.com/windmilleng/tilt/internal/k8s"
"github.com/windmilleng/tilt/internal/k8s/testyaml"
)
Expand All @@ -18,8 +18,8 @@ func TestInjectSyncletSidecar(t *testing.T) {

assert.Equal(t, 1, len(entities))
entity := entities[0]
name, _ := reference.ParseNamed("gcr.io/some-project-162817/sancho")
newEntity, replaced, err := InjectSyncletSidecar(entity, name)
selector := container.MustParseSelector("gcr.io/some-project-162817/sancho")
newEntity, replaced, err := InjectSyncletSidecar(entity, selector)
if err != nil {
t.Fatal(err)
} else if !replaced {
Expand Down
Loading

0 comments on commit 30ef55a

Please sign in to comment.