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.

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

Show warning when there are no images to pull #42

Merged
merged 4 commits into from
Jan 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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)
}

alemorcuq marked this conversation as resolved.
Show resolved Hide resolved
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: []