Skip to content

Commit

Permalink
Show warning when there are no images to pull (#42)
Browse files Browse the repository at this point in the history
* Show warning when there are no images to pull

The output of `dt wrap` and `dt images pull` shows the following message
when there are no images to pull:

```
       ✔  All images pulled successfully
```

Instead, we are going to show a warning when there are no images to pull:

```
       ⚠️  No images found in Images.lock
```

* Add missing scenario

* Improve error flow

* Don't try to pull images if there are no images to pull
  • Loading branch information
alemorcuq authored Jan 23, 2024
1 parent 79139ce commit b167558
Show file tree
Hide file tree
Showing 15 changed files with 165 additions and 60 deletions.
2 changes: 1 addition & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ run:

linters-settings:
gocyclo:
min-complexity: 16
min-complexity: 18
govet:
check-shadowing: false
lll:
Expand Down
44 changes: 25 additions & 19 deletions cmd/dt/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,16 @@ import (
"github.com/spf13/cobra"
"github.com/vmware-labs/distribution-tooling-for-helm/internal/log"
"github.com/vmware-labs/distribution-tooling-for-helm/pkg/chartutils"
"github.com/vmware-labs/distribution-tooling-for-helm/pkg/imagelock"
"github.com/vmware-labs/distribution-tooling-for-helm/pkg/utils"
"github.com/vmware-labs/distribution-tooling-for-helm/pkg/wrapping"
)

var pullCmd = newPullCommand()

func pullChartImages(chart wrapping.Lockable, imagesDir string, opts ...chartutils.Option) error {
lockFile := chart.LockFilePath()

lock, err := imagelock.FromYAMLFile(lockFile)
lock, err := chart.GetImagesLock()
if err != nil {
return fmt.Errorf("failed to read Images.lock file")
return fmt.Errorf("failed to read Images.lock file: %v", err)
}
if err := chartutils.PullImages(lock, imagesDir,
opts...,
Expand Down Expand Up @@ -63,21 +60,30 @@ func newPullCommand() *cobra.Command {
if imagesDir == "" {
imagesDir = chart.ImagesDir()
}
if err := l.Section(fmt.Sprintf("Pulling images into %q", chart.ImagesDir()), func(childLog log.SectionLogger) error {
if err := pullChartImages(
chart,
imagesDir,
chartutils.WithLog(childLog),
chartutils.WithContext(ctx),
chartutils.WithProgressBar(childLog.ProgressBar()),
chartutils.WithArtifactsDir(chart.ImageArtifactsDir()),
); err != nil {
return childLog.Failf("%v", err)
lock, err := chart.GetImagesLock()
if err != nil {
return l.Failf("Failed to load Images.lock: %v", err)
}

if len(lock.Images) == 0 {
l.Warnf("No images found in Images.lock")
} else {
if err := l.Section(fmt.Sprintf("Pulling images into %q", chart.ImagesDir()), func(childLog log.SectionLogger) error {
if err := pullChartImages(
chart,
imagesDir,
chartutils.WithLog(childLog),
chartutils.WithContext(ctx),
chartutils.WithProgressBar(childLog.ProgressBar()),
chartutils.WithArtifactsDir(chart.ImageArtifactsDir()),
); err != nil {
return childLog.Failf("%v", err)
}
childLog.Infof("All images pulled successfully")
return nil
}); err != nil {
return l.Failf("%w", err)
}
childLog.Infof("All images pulled successfully")
return nil
}); err != nil {
return l.Failf("%w", err)
}

if outputFile != "" {
Expand Down
11 changes: 10 additions & 1 deletion cmd/dt/pull_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,21 @@ func (suite *CmdSuite) TestPullCommand() {
verifyChartDir(tmpDir)
})

t.Run("Warning when no images in Images.lock", func(t *testing.T) {
images = []tu.ImageData{}
scenarioName := "no-images-chart"
scenarioDir = fmt.Sprintf("../../testdata/scenarios/%s", scenarioName)
chartDir := createSampleChart(sb.TempFile())
dt("images", "pull", chartDir).AssertSuccessMatch(t, "No images found in Images.lock")
require.NoDirExists(filepath.Join(chartDir, "images"))
})

t.Run("Errors", func(t *testing.T) {
t.Run("Fails when Images.lock is not found", func(t *testing.T) {
chartDir := createSampleChart(sb.TempFile())
require.NoError(os.RemoveAll(filepath.Join(chartDir, "Images.lock")))

dt("images", "pull", chartDir).AssertErrorMatch(t, `(?s).*failed to read Images\.lock file.*`)
dt("images", "pull", chartDir).AssertErrorMatch(t, `Failed to load Images\.lock.*`)
})
})
}
12 changes: 1 addition & 11 deletions cmd/dt/push.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,27 +3,17 @@ package main
import (
"context"
"fmt"
"os"

"github.com/spf13/cobra"
"github.com/vmware-labs/distribution-tooling-for-helm/internal/log"
"github.com/vmware-labs/distribution-tooling-for-helm/pkg/chartutils"
"github.com/vmware-labs/distribution-tooling-for-helm/pkg/imagelock"
"github.com/vmware-labs/distribution-tooling-for-helm/pkg/wrapping"
)

var pushCmd = newPushCmd()

func pushChartImages(wrap wrapping.Wrap, imagesDir string, opts ...chartutils.Option) error {
lockFile := wrap.LockFilePath()

fh, err := os.Open(lockFile)
if err != nil {
return fmt.Errorf("failed to open Images.lock file: %v", err)
}
defer fh.Close()

lock, err := imagelock.FromYAML(fh)
lock, err := wrap.GetImagesLock()
if err != nil {
return fmt.Errorf("failed to load Images.lock: %v", err)
}
Expand Down
3 changes: 1 addition & 2 deletions cmd/dt/unwrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"github.com/vmware-labs/distribution-tooling-for-helm/internal/widgets"
"github.com/vmware-labs/distribution-tooling-for-helm/pkg/artifacts"
"github.com/vmware-labs/distribution-tooling-for-helm/pkg/chartutils"
"github.com/vmware-labs/distribution-tooling-for-helm/pkg/imagelock"
"github.com/vmware-labs/distribution-tooling-for-helm/pkg/relocator"
"github.com/vmware-labs/distribution-tooling-for-helm/pkg/utils"
"github.com/vmware-labs/distribution-tooling-for-helm/pkg/wrapping"
Expand Down Expand Up @@ -155,7 +154,7 @@ func pushChartImagesAndVerify(ctx context.Context, wrap wrapping.Wrap, l log.Sec
}

func showImagesSummary(wrap wrapping.Lockable, l log.SectionLogger) int {
lock, err := imagelock.FromYAMLFile(wrap.LockFilePath())
lock, err := wrap.GetImagesLock()
if err != nil {
l.Debugf("failed to load list of images: failed to load lock file: %v", err)
return 0
Expand Down
41 changes: 25 additions & 16 deletions cmd/dt/wrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,23 +111,32 @@ func wrapChart(ctx context.Context, inputPath string, outputFile string, platfor
}
}

if err := l.Section(fmt.Sprintf("Pulling images into %q", chart.ImagesDir()), func(childLog log.SectionLogger) error {
fetchArtifacts, _ := flags.GetBool("fetch-artifacts")
if err := pullChartImages(
wrap,
wrap.ImagesDir(),
chartutils.WithLog(childLog),
chartutils.WithContext(ctx),
chartutils.WithFetchArtifacts(fetchArtifacts),
chartutils.WithArtifactsDir(wrap.ImageArtifactsDir()),
chartutils.WithProgressBar(childLog.ProgressBar()),
); err != nil {
return childLog.Failf("%v", err)
lock, err := chart.GetImagesLock()
if err != nil {
return l.Failf("Failed to load Images.lock: %v", err)
}

if len(lock.Images) == 0 {
l.Warnf("No images found in Images.lock")
} else {
if err := l.Section(fmt.Sprintf("Pulling images into %q", chart.ImagesDir()), func(childLog log.SectionLogger) error {
fetchArtifacts, _ := flags.GetBool("fetch-artifacts")
if err := pullChartImages(
wrap,
wrap.ImagesDir(),
chartutils.WithLog(childLog),
chartutils.WithContext(ctx),
chartutils.WithFetchArtifacts(fetchArtifacts),
chartutils.WithArtifactsDir(wrap.ImageArtifactsDir()),
chartutils.WithProgressBar(childLog.ProgressBar()),
); err != nil {
return childLog.Failf("%v", err)
}
childLog.Infof("All images pulled successfully")
return nil
}); err != nil {
return err
}
childLog.Infof("All images pulled successfully")
return nil
}); err != nil {
return err
}

if shouldCarvelize(flags) {
Expand Down
32 changes: 24 additions & 8 deletions cmd/dt/wrap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,18 +111,27 @@ func testChartWrap(t *testing.T, sb *tu.Sandbox, inputChart string, expectedLock
if cfg.FetchArtifacts {
args = append(args, "--fetch-artifacts")
}
dt(args...).AssertSuccess(t)

if len(cfg.Images) == 0 {
dt(args...).AssertSuccessMatch(t, "No images found in Images.lock")
} else {
dt(args...).AssertSuccess(t)
}
require.FileExists(t, expectedWrapFile)

tmpDir := sb.TempFile()
require.NoError(t, utils.Untar(expectedWrapFile, tmpDir, utils.TarConfig{StripComponents: 1}))

imagesDir := filepath.Join(tmpDir, "images")
require.DirExists(t, imagesDir)
for _, imgData := range cfg.Images {
for _, digestData := range imgData.Digests {
imgDir := filepath.Join(imagesDir, fmt.Sprintf("%s.layout", digestData.Digest.Encoded()))
assert.DirExists(t, imgDir)
if len(cfg.Images) == 0 {
require.NoDirExists(t, imagesDir)
} else {
require.DirExists(t, imagesDir)
for _, imgData := range cfg.Images {
for _, digestData := range imgData.Digests {
imgDir := filepath.Join(imagesDir, fmt.Sprintf("%s.layout", digestData.Digest.Encoded()))
assert.DirExists(t, imgDir)
}
}
}
wrappedChartDir := filepath.Join(tmpDir, "chart")
Expand Down Expand Up @@ -247,10 +256,10 @@ func (suite *CmdSuite) TestWrapCommand() {
testWrap(t, chartDir, outputFile, expectedLock, generateCarvelBundle, fetchArtifacts)
}

t.Run("Wrap Chart without exiting lock", func(t *testing.T) {
t.Run("Wrap Chart without existing lock", func(t *testing.T) {
testSampleWrap(t, withoutLock, "", false, WithoutArtifacts)
})
t.Run("Wrap Chart with exiting lock", func(t *testing.T) {
t.Run("Wrap Chart with existing lock", func(t *testing.T) {
testSampleWrap(t, withLock, "", false, WithoutArtifacts)
})
t.Run("Wrap Chart From compressed tgz", func(t *testing.T) {
Expand Down Expand Up @@ -323,4 +332,11 @@ func (suite *CmdSuite) TestWrapCommand() {
tempFilename := fmt.Sprintf("%s/chart.wrap.tar.gz", sb.TempFile())
testSampleWrap(t, withLock, tempFilename, true, WithoutArtifacts) // triggers the Carvel checks
})

t.Run("Wrap Chart with no images", func(t *testing.T) {
images = []tu.ImageData{}
scenarioName = "no-images-chart"
scenarioDir = fmt.Sprintf("../../testdata/scenarios/%s", scenarioName)
testSampleWrap(t, withLock, "", false, WithoutArtifacts)
})
}
12 changes: 12 additions & 0 deletions pkg/chartutils/chart.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,18 @@ func (c *Chart) LockFilePath() string {
return c.AbsFilePath(imagelock.DefaultImagesLockFileName)
}

// GetImagesLock returns the chart's ImagesLock object
func (c *Chart) GetImagesLock() (*imagelock.ImagesLock, error) {
lockFile := c.LockFilePath()

lock, err := imagelock.FromYAMLFile(lockFile)
if err != nil {
return nil, err
}

return lock, nil
}

// ImageArtifactsDir returns the imags artifacts directory
func (c *Chart) ImageArtifactsDir() string {
return filepath.Join(c.RootDir(), artifacts.HelmArtifactsFolder, "images")
Expand Down
4 changes: 4 additions & 0 deletions pkg/chartutils/images.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ func PullImages(lock *imagelock.ImagesLock, imagesDir string, opts ...Option) er
}
l := cfg.Log

if len(lock.Images) == 0 {
return fmt.Errorf("no images found in Images.lock")
}

p, _ := cfg.ProgressBar.WithTotal(getNumberOfArtifacts(lock.Images)).UpdateTitle("Pulling Images").Start()
defer p.Stop()
maxRetries := cfg.MaxRetries
Expand Down
29 changes: 28 additions & 1 deletion pkg/chartutils/images_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func (suite *ChartUtilsTestSuite) TestPullImages() {

scenarioDir := fmt.Sprintf("../../testdata/scenarios/%s", scenarioName)

suite.T().Run("Pulls images", func(t *testing.T) {
t.Run("Pulls images", func(t *testing.T) {
chartDir := sb.TempFile()

require.NoError(tu.RenderScenario(scenarioDir, chartDir,
Expand All @@ -66,6 +66,33 @@ func (suite *ChartUtilsTestSuite) TestPullImages() {
}
}
})

t.Run("Error when no images in Images.lock", func(t *testing.T) {
chartDir := sb.TempFile()

images := []tu.ImageData{}
scenarioName := "no-images-chart"
scenarioDir := fmt.Sprintf("../../testdata/scenarios/%s", scenarioName)
require.NoError(tu.RenderScenario(scenarioDir, chartDir,
map[string]interface{}{"ServerURL": serverURL, "Images": images, "Name": chartName, "RepositoryURL": serverURL},
))
imagesDir := filepath.Join(chartDir, "images")

require.NoError(err)

lock, err := imagelock.FromYAMLFile(filepath.Join(chartDir, "Images.lock"))
require.NoError(err)
require.Error(PullImages(lock, imagesDir))

require.DirExists(imagesDir)

for _, imgData := range images {
for _, digestData := range imgData.Digests {
imgDir := filepath.Join(imagesDir, fmt.Sprintf("%s.layout", digestData.Digest.Encoded()))
suite.Assert().DirExists(imgDir)
}
}
})
}

func (suite *ChartUtilsTestSuite) TestPushImages() {
Expand Down
2 changes: 1 addition & 1 deletion pkg/imagelock/lock.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func FromYAML(r io.Reader) (*ImagesLock, error) {
func FromYAMLFile(file string) (*ImagesLock, error) {
fh, err := os.Open(file)
if err != nil {
return nil, fmt.Errorf("failed to open Images.lock file: %v", err)
return nil, fmt.Errorf("failed to open Images.lock file: %w", err)
}
defer fh.Close()
return FromYAML(fh)
Expand Down
6 changes: 6 additions & 0 deletions pkg/wrapping/wrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
type Lockable interface {
LockFilePath() string
ImagesDir() string
GetImagesLock() (*imagelock.ImagesLock, error)
}

// Wrap defines the interface to implement a Helm chart wrap
Expand Down Expand Up @@ -55,6 +56,11 @@ func (w *wrap) ImagesDir() string {
return w.AbsFilePath("images")
}

// GetImagesLock returns the chart's ImagesLock object
func (w *wrap) GetImagesLock() (*imagelock.ImagesLock, error) {
return w.chart.GetImagesLock()
}

// AbsFilePath returns the absolute path to the Chart relative file name
func (w *wrap) AbsFilePath(name string) string {
return filepath.Join(w.rootDir, name)
Expand Down
16 changes: 16 additions & 0 deletions testdata/scenarios/no-images-chart/Chart.yaml.tmpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
name: {{or .Name "WordPress"}}
version: {{or .Version "1.0.0"}}
annotations:
category: CMS
licenses: Apache-2.0
appVersion: {{if .AppVersion}}{{.AppVersion}}{{else}}6.2.2{{end}}
{{if .Dependencies }}
{{if gt (len .Dependencies) 0 }}
dependencies:
{{- range .Dependencies}}
- name: {{.Name}}
repository: {{.Repository}}
version: {{.Version}}
{{end -}}
{{end}}
{{end}}
1 change: 1 addition & 0 deletions testdata/scenarios/no-images-chart/Images.lock.tmpl
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{{include "imagelock.partial.tmpl" . }}
10 changes: 10 additions & 0 deletions testdata/scenarios/no-images-chart/imagelock.partial.tmpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
apiVersion: v0
kind: ImagesLock
metadata:
generatedAt: "2023-07-13T16:30:33.284125307Z"
generatedBy: Distribution Tooling for Helm
chart:
name: {{.Name}}
version: 1.0.0
appVersion: {{if .AppVersion}}{{.AppVersion}}{{else}}6.2.2{{end}}
images: []

0 comments on commit b167558

Please sign in to comment.