Skip to content

Commit

Permalink
Merge pull request #999 from FabianKramm/fixes
Browse files Browse the repository at this point in the history
fix issue with wrongly cached image tags (#998)
  • Loading branch information
FabianKramm authored Mar 4, 2020
2 parents bc8c308 + a8ed05b commit 79c0e28
Show file tree
Hide file tree
Showing 5 changed files with 168 additions and 187 deletions.
53 changes: 2 additions & 51 deletions pkg/devspace/deploy/deployer/helm/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,9 @@ import (
yaml "gopkg.in/yaml.v2"

"github.com/devspace-cloud/devspace/pkg/devspace/config/generated"
"github.com/devspace-cloud/devspace/pkg/devspace/config/versions/latest"
"github.com/devspace-cloud/devspace/pkg/devspace/deploy/deployer/helm/merge"
"github.com/devspace-cloud/devspace/pkg/devspace/deploy/deployer/kubectl/walk"
"github.com/devspace-cloud/devspace/pkg/devspace/deploy/deployer/util"
"github.com/devspace-cloud/devspace/pkg/devspace/helm"
"github.com/devspace-cloud/devspace/pkg/devspace/registry"
hashpkg "github.com/devspace-cloud/devspace/pkg/util/hash"
"github.com/devspace-cloud/devspace/pkg/util/yamlutil"
"github.com/mgutz/ansi"
Expand Down Expand Up @@ -151,7 +149,7 @@ func (d *DeployConfig) internalDeploy(cache *generated.CacheConfig, forceDeploy
// Add devspace specific values
if d.DeploymentConfig.Helm.ReplaceImageTags == nil || *d.DeploymentConfig.Helm.ReplaceImageTags == true {
// Replace image names
shouldRedeploy := replaceContainerNames(overwriteValues, cache, d.config.Images, builtImages)
shouldRedeploy := util.ReplaceImageNames(overwriteValues, cache, d.config.Images, builtImages, nil)
if forceDeploy == false && shouldRedeploy {
forceDeploy = true
}
Expand Down Expand Up @@ -191,50 +189,3 @@ func (d *DeployConfig) internalDeploy(cache *generated.CacheConfig, forceDeploy

return true, nil
}

func replaceContainerNames(overwriteValues map[interface{}]interface{}, cache *generated.CacheConfig, imagesConf map[string]*latest.ImageConfig, builtImages map[string]string) bool {
shouldRedeploy := false

match := func(path, key, value string) bool {
image, err := registry.GetStrippedDockerImageName(value)
if err != nil {
return false
}

// Search for image name
for _, imageCache := range cache.Images {
if imageCache.ImageName == image && imageCache.Tag != "" {
if builtImages != nil {
if _, ok := builtImages[image]; ok {
shouldRedeploy = true
}
}

return true
}
}

return false
}

replace := func(path, value string) (interface{}, error) {
image, err := registry.GetStrippedDockerImageName(value)
if err != nil {
return false, nil
}

// Search for image name
for _, imageCache := range cache.Images {
if imageCache.ImageName == image {
return image + ":" + imageCache.Tag, nil
}
}

return value, nil
}

// We ignore the error here because our replace function never throws an error
_ = walk.Walk(overwriteValues, match, replace)

return shouldRedeploy
}
76 changes: 0 additions & 76 deletions pkg/devspace/deploy/deployer/helm/deploy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,79 +124,3 @@ func TestDeploy(t *testing.T) {
assert.Equal(t, string(cacheAsYaml), string(expectationAsYaml), "Unexpected cache in testCase %s", testCase.name)
}
}

type replaceContainerNamesTestCase struct {
name string

overwriteValues map[interface{}]interface{}
cache *generated.CacheConfig
imagesConf map[string]*latest.ImageConfig
builtImages map[string]string

expectedShouldRedeploy bool
expectedOverwriteValues map[interface{}]interface{}
}

func TestReplaceContainerNames(t *testing.T) {
testCases := []replaceContainerNamesTestCase{
replaceContainerNamesTestCase{
name: "invalid image name",
overwriteValues: map[interface{}]interface{}{
"": "",
},
cache: &generated.CacheConfig{
Images: map[string]*generated.ImageCache{
"": &generated.ImageCache{},
},
},
expectedOverwriteValues: map[interface{}]interface{}{
"": "",
},
},
replaceContainerNamesTestCase{
name: "Image not in cache",
overwriteValues: map[interface{}]interface{}{
"": "myimage",
},
cache: &generated.CacheConfig{
Images: map[string]*generated.ImageCache{},
},
expectedOverwriteValues: map[interface{}]interface{}{
"": "myimage",
},
},
replaceContainerNamesTestCase{
name: "Image in cache",
overwriteValues: map[interface{}]interface{}{
"": "myimage",
},
cache: &generated.CacheConfig{
Images: map[string]*generated.ImageCache{
"": &generated.ImageCache{
ImageName: "myimage",
Tag: "someTag",
},
},
},
builtImages: map[string]string{
"myimage": "",
},
expectedShouldRedeploy: true,
expectedOverwriteValues: map[interface{}]interface{}{
"": "myimage:someTag",
},
},
}

for _, testCase := range testCases {
shouldRedeploy := replaceContainerNames(testCase.overwriteValues, testCase.cache, testCase.imagesConf, testCase.builtImages)

assert.Equal(t, shouldRedeploy, testCase.expectedShouldRedeploy, "Unexpected deployed-bool in testCase %s", testCase.name)

ovAsYaml, err := yaml.Marshal(testCase.overwriteValues)
assert.NilError(t, err, "Error marshaling overwriteValues in testCase %s", testCase.name)
expectationAsYaml, err := yaml.Marshal(testCase.expectedOverwriteValues)
assert.NilError(t, err, "Error marshaling expectation in testCase %s", testCase.name)
assert.Equal(t, string(ovAsYaml), string(expectationAsYaml), "Unexpected overwriteValues in testCase %s", testCase.name)
}
}
62 changes: 2 additions & 60 deletions pkg/devspace/deploy/deployer/kubectl/kubectl.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,8 @@ import (

"github.com/devspace-cloud/devspace/pkg/devspace/config/generated"
"github.com/devspace-cloud/devspace/pkg/devspace/deploy/deployer"
"github.com/devspace-cloud/devspace/pkg/devspace/deploy/deployer/kubectl/walk"
"github.com/devspace-cloud/devspace/pkg/devspace/deploy/deployer/util"
"github.com/devspace-cloud/devspace/pkg/devspace/kubectl"
"github.com/devspace-cloud/devspace/pkg/devspace/registry"

"github.com/devspace-cloud/devspace/pkg/devspace/config/versions/latest"
"github.com/devspace-cloud/devspace/pkg/util/hash"
Expand Down Expand Up @@ -252,7 +251,7 @@ func (d *DeployConfig) getReplacedManifest(manifest string, cache *generated.Cac

if len(cache.Images) > 0 {
if d.DeploymentConfig.Kubectl.ReplaceImageTags == nil || *d.DeploymentConfig.Kubectl.ReplaceImageTags == true {
shouldRedeploy = replaceManifest(manifestYaml, cache, d.config.Images, builtImages) || shouldRedeploy
shouldRedeploy = util.ReplaceImageNames(manifestYaml, cache, d.config.Images, builtImages, map[string]bool{"image": true}) || shouldRedeploy
}
}

Expand Down Expand Up @@ -319,60 +318,3 @@ func (d *DeployConfig) dryRun(manifest string) ([]byte, error) {

return output, nil
}

func replaceManifest(manifest map[interface{}]interface{}, cache *generated.CacheConfig, imagesConf map[string]*latest.ImageConfig, builtImages map[string]string) bool {
shouldRedeploy := false

match := func(path, key, value string) bool {
if key == "image" {
image, err := registry.GetStrippedDockerImageName(value)
if err != nil {
return false
}

// Search for image name
for _, imageCache := range cache.Images {
found := false
for _, imageConfig := range imagesConf {
if imageConfig.Image == image {
found = true
break
}
}

if found && imageCache.ImageName == image && imageCache.Tag != "" {
if builtImages != nil {
if _, ok := builtImages[image]; ok {
shouldRedeploy = true
}
}

return true
}
}
}

return false
}

replace := func(path, value string) (interface{}, error) {
image, err := registry.GetStrippedDockerImageName(value)
if err != nil {
return false, nil
}

// Search for image name
for _, imageCache := range cache.Images {
if imageCache.ImageName == image {
return image + ":" + imageCache.Tag, nil
}
}

return value, nil
}

// We ignore the error here because the replace function can never throw an error
_ = walk.Walk(manifest, match, replace)

return shouldRedeploy
}
75 changes: 75 additions & 0 deletions pkg/devspace/deploy/deployer/util/replace.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
package util

import (
"github.com/devspace-cloud/devspace/pkg/devspace/config/generated"
"github.com/devspace-cloud/devspace/pkg/devspace/config/versions/latest"
"github.com/devspace-cloud/devspace/pkg/devspace/deploy/deployer/kubectl/walk"
"github.com/devspace-cloud/devspace/pkg/devspace/registry"
)

// ReplaceImageNames replaces images within a certain manifest with the correct tags from the cache
func ReplaceImageNames(manifest map[interface{}]interface{}, cache *generated.CacheConfig, imagesConf map[string]*latest.ImageConfig, builtImages map[string]string, keys map[string]bool) bool {
if imagesConf == nil {
imagesConf = map[string]*latest.ImageConfig{}
}
if keys == nil {
keys = map[string]bool{}
}

// strip out images from cache that are not in the imagesconf anymore
for name := range cache.Images {
if _, ok := imagesConf[name]; !ok {
delete(cache.Images, name)
}
}

shouldRedeploy := false

match := func(path, key, value string) bool {
if len(keys) > 0 && keys[key] == false {
return false
}

// Strip tag from image
image, err := registry.GetStrippedDockerImageName(value)
if err != nil {
return false
}

// Search for image name
for _, imageCache := range cache.Images {
if imageCache.ImageName == image && imageCache.Tag != "" {
if builtImages != nil {
if _, ok := builtImages[image]; ok {
shouldRedeploy = true
}
}

return true
}
}

return false
}

replace := func(path, value string) (interface{}, error) {
image, err := registry.GetStrippedDockerImageName(value)
if err != nil {
return false, nil
}

// Search for image name
for _, imageCache := range cache.Images {
if imageCache.ImageName == image {
return image + ":" + imageCache.Tag, nil
}
}

return value, nil
}

// We ignore the error here because the replace function can never throw an error
_ = walk.Walk(manifest, match, replace)

return shouldRedeploy
}
89 changes: 89 additions & 0 deletions pkg/devspace/deploy/deployer/util/replace_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
package util

import (
"testing"

"github.com/devspace-cloud/devspace/pkg/devspace/config/generated"
"github.com/devspace-cloud/devspace/pkg/devspace/config/versions/latest"
"gopkg.in/yaml.v2"
"gotest.tools/assert"
)

type replaceContainerNamesTestCase struct {
name string

overwriteValues map[interface{}]interface{}
cache *generated.CacheConfig
imagesConf map[string]*latest.ImageConfig
builtImages map[string]string

expectedShouldRedeploy bool
expectedOverwriteValues map[interface{}]interface{}
}

func TestReplaceContainerNames(t *testing.T) {
testCases := []replaceContainerNamesTestCase{
replaceContainerNamesTestCase{
name: "invalid image name",
overwriteValues: map[interface{}]interface{}{
"": "",
},
cache: &generated.CacheConfig{
Images: map[string]*generated.ImageCache{
"": &generated.ImageCache{},
},
},
expectedOverwriteValues: map[interface{}]interface{}{
"": "",
},
},
replaceContainerNamesTestCase{
name: "Image not in cache",
overwriteValues: map[interface{}]interface{}{
"": "myimage",
},
cache: &generated.CacheConfig{
Images: map[string]*generated.ImageCache{},
},
expectedOverwriteValues: map[interface{}]interface{}{
"": "myimage",
},
},
replaceContainerNamesTestCase{
name: "Image in cache",
overwriteValues: map[interface{}]interface{}{
"": "myimage",
},
imagesConf: map[string]*latest.ImageConfig{
"test": &latest.ImageConfig{},
},
cache: &generated.CacheConfig{
Images: map[string]*generated.ImageCache{
"test": &generated.ImageCache{
ImageName: "myimage",
Tag: "someTag",
},
},
},
builtImages: map[string]string{
"myimage": "",
},
expectedShouldRedeploy: true,
expectedOverwriteValues: map[interface{}]interface{}{
"": "myimage:someTag",
},
},
}

for _, testCase := range testCases {
shouldRedeploy := ReplaceImageNames(testCase.overwriteValues, testCase.cache, testCase.imagesConf, testCase.builtImages, nil)

assert.Equal(t, shouldRedeploy, testCase.expectedShouldRedeploy, "Unexpected deployed-bool in testCase %s", testCase.name)

ovAsYaml, err := yaml.Marshal(testCase.overwriteValues)
assert.NilError(t, err, "Error marshaling overwriteValues in testCase %s", testCase.name)
expectationAsYaml, err := yaml.Marshal(testCase.expectedOverwriteValues)
assert.NilError(t, err, "Error marshaling expectation in testCase %s", testCase.name)
assert.Equal(t, string(ovAsYaml), string(expectationAsYaml), "Unexpected overwriteValues in testCase %s", testCase.name)
}
}

0 comments on commit 79c0e28

Please sign in to comment.