From f9e9577a744e4ceb4dc7ab7da356ce5ca56de73d Mon Sep 17 00:00:00 2001 From: Parthiba-Hazra Date: Sun, 18 Feb 2024 00:24:40 +0530 Subject: [PATCH 01/11] initial commit: Add basic functionality for pull-policy (eg; --pull-policy interval=2d5h20m) Signed-off-by: Parthiba-Hazra --- Makefile | 10 +++ pkg/image/fetcher.go | 59 +++++++++++++++++- pkg/image/pull_policy.go | 130 ++++++++++++++++++++++++++++++++++++++- 3 files changed, 197 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 0d1246ea27..f365fbcdff 100644 --- a/Makefile +++ b/Makefile @@ -33,6 +33,10 @@ TEST_TIMEOUT?=1200s UNIT_TIMEOUT?=$(TEST_TIMEOUT) NO_DOCKER?= +# Define the directory for the image.json file +IMAGE_JSON_DIR := $(HOME)/.pack +IMAGE_JSON_FILE := $(IMAGE_JSON_DIR)/image.json + # Clean build flags clean_build := $(strip ${PACK_BUILD}) clean_sha := $(strip ${PACK_GITSHA1}) @@ -146,6 +150,12 @@ package: out install: mkdir -p ${DESTDIR}${BINDIR} cp ./out/$(PACK_BIN) ${DESTDIR}${BINDIR}/ + @if [ ! -d "$(IMAGE_JSON_DIR)" ]; then \ + mkdir -p $(IMAGE_JSON_DIR); \ + fi + @if [ ! -f "$(IMAGE_JSON_FILE)" ]; then \ + echo '{ "interval": { "duration": "" }, "image": {} }' > $(IMAGE_JSON_FILE); \ + fi ## install-mockgen: Used only by apt-get install when installing ubuntu ppa install-mockgen: diff --git a/pkg/image/fetcher.go b/pkg/image/fetcher.go index 60548fcc1d..ce813d8ace 100644 --- a/pkg/image/fetcher.go +++ b/pkg/image/fetcher.go @@ -5,7 +5,9 @@ import ( "encoding/base64" "encoding/json" "io" + "os" "strings" + "time" "github.com/buildpacks/imgutil/layout" "github.com/buildpacks/imgutil/layout/sparse" @@ -35,6 +37,15 @@ type LayoutOption struct { Sparse bool } +type ImageJSON struct { + Interval struct { + Duration string `json:"duration"` + } `json:"interval"` + Image struct { + ImageIDtoTIME map[string]string + } `json:"image"` +} + // WithRegistryMirrors supply your own mirrors for registry. func WithRegistryMirrors(registryMirrors map[string]string) FetcherOption { return func(c *Fetcher) { @@ -89,6 +100,14 @@ func (f *Fetcher) Fetch(ctx context.Context, name string, options FetchOptions) return nil, err } + var imageID string // Variable to store the image ID + parts := strings.Split(name, "@") + if len(parts) > 1 { + imageID = parts[1] + } else { + imageID = parts[0] + } + if (options.LayoutOption != LayoutOption{}) { return f.fetchLayoutImage(name, options.LayoutOption) } @@ -106,6 +125,15 @@ func (f *Fetcher) Fetch(ctx context.Context, name string, options FetchOptions) if err == nil || !errors.Is(err, ErrNotFound) { return img, err } + case PullWithInterval: + pull, err := CheckImagePullInterval(imageID) + if !pull { + if err != nil { + return nil, err + } + img, err := f.fetchDaemonImage(name) + return img, err + } } f.logger.Debugf("Pulling image %s", style.Symbol(name)) @@ -120,7 +148,17 @@ func (f *Fetcher) Fetch(ctx context.Context, name string, options FetchOptions) return nil, err } - return f.fetchDaemonImage(name) + image, err := f.fetchDaemonImage(name) + if err != nil { + return nil, err + } + + // Update image pull record in the JSON file + if err := f.updateImagePullRecord(imageID, time.Now().Format(time.RFC3339)); err != nil { + return nil, err + } + + return image, nil } func (f *Fetcher) fetchDaemonImage(name string) (imgutil.Image, error) { @@ -246,3 +284,22 @@ func (w *colorizedWriter) Write(p []byte) (n int, err error) { } return w.writer.Write([]byte(msg)) } + +func (f *Fetcher) updateImagePullRecord(imageID, timestamp string) error { + imageJSON, err := readImageJSON() + if err != nil { + return err + } + + if imageJSON.Image.ImageIDtoTIME == nil { + imageJSON.Image.ImageIDtoTIME = make(map[string]string) + } + imageJSON.Image.ImageIDtoTIME[imageID] = timestamp + + updatedJSON, err := json.MarshalIndent(imageJSON, "", " ") + if err != nil { + return errors.Wrap(err, "failed to marshal updated records") + } + + return os.WriteFile(imagePath, updatedJSON, 0644) +} diff --git a/pkg/image/pull_policy.go b/pkg/image/pull_policy.go index f08678ac5a..13af83cc92 100644 --- a/pkg/image/pull_policy.go +++ b/pkg/image/pull_policy.go @@ -1,12 +1,28 @@ package image import ( + "encoding/json" + "fmt" + "os" + "path/filepath" + "regexp" + "strconv" + "strings" + "time" + "github.com/pkg/errors" ) // PullPolicy defines a policy for how to manage images type PullPolicy int +var interval time.Duration + +var ( + intervalRegex = regexp.MustCompile(`^(\d+d)?(\d+h)?(\d+m)?$`) + imagePath string +) + const ( // PullAlways images, even if they are present PullAlways PullPolicy = iota @@ -14,16 +30,30 @@ const ( PullNever // PullIfNotPresent pulls images if they aren't present PullIfNotPresent + // PullWithInterval pulls images with specified intervals + PullWithInterval ) var nameMap = map[string]PullPolicy{"always": PullAlways, "never": PullNever, "if-not-present": PullIfNotPresent, "": PullAlways} -// ParsePullPolicy from string +// ParsePullPolicy from string with support for interval formats func ParsePullPolicy(policy string) (PullPolicy, error) { if val, ok := nameMap[policy]; ok { return val, nil } + if strings.HasPrefix(policy, "interval=") { + intervalStr := strings.TrimPrefix(policy, "interval=") + matches := intervalRegex.FindStringSubmatch(intervalStr) + if len(matches) == 0 { + return PullAlways, errors.Errorf("invalid interval format: %s", intervalStr) + } + + updateImageJSONDuration(intervalStr) + + return PullWithInterval, nil + } + return PullAlways, errors.Errorf("invalid pull policy %s", policy) } @@ -35,7 +65,105 @@ func (p PullPolicy) String() string { return "never" case PullIfNotPresent: return "if-not-present" + case PullWithInterval: + return fmt.Sprintf("interval=%v", interval) } return "" } + +func updateImageJSONDuration(intervalStr string) error { + imageJSON, err := readImageJSON() + if err != nil { + return err + } + + imageJSON.Interval.Duration = intervalStr + + updatedJSON, err := json.MarshalIndent(imageJSON, "", " ") + if err != nil { + return errors.Wrap(err, "failed to marshal updated records") + } + + return os.WriteFile(imagePath, updatedJSON, 0644) +} + +func readImageJSON() (ImageJSON, error) { + homeDir, err := os.UserHomeDir() + if err != nil { + return ImageJSON{}, errors.Wrap(err, "failed to get home directory") + } + imagePath = filepath.Join(homeDir, ".pack", "image.json") + + jsonData, err := os.ReadFile(imagePath) + if err != nil && !os.IsNotExist(err) { + return ImageJSON{}, errors.Wrap(err, "failed to read image.json") + } + + var imageJSON ImageJSON + if err := json.Unmarshal(jsonData, &imageJSON); err != nil && !os.IsNotExist(err) { + return ImageJSON{}, errors.Wrap(err, "failed to unmarshal image.json") + } + + return imageJSON, nil +} + +func CheckImagePullInterval(imageID string) (bool, error) { + imageJSON, err := readImageJSON() + if err != nil { + return false, err + } + + timestamp, ok := imageJSON.Image.ImageIDtoTIME[imageID] + if !ok { + // If the image ID is not present, return true + return true, nil + } + + imageTimestamp, err := time.Parse(time.RFC3339, timestamp) + if err != nil { + return false, errors.Wrap(err, "failed to parse image timestamp from JSON") + } + + durationStr := imageJSON.Interval.Duration + + duration, err := parseDurationString(durationStr) + if err != nil { + return false, errors.Wrap(err, "failed to parse duration from JSON") + } + + timeThreshold := time.Now().Add(-duration) + + return imageTimestamp.Before(timeThreshold), nil +} + +func parseDurationString(durationStr string) (time.Duration, error) { + var totalMinutes int + for i := 0; i < len(durationStr); { + endIndex := i + 1 + for endIndex < len(durationStr) && durationStr[endIndex] >= '0' && durationStr[endIndex] <= '9' { + endIndex++ + } + + value, err := strconv.Atoi(durationStr[i:endIndex]) + if err != nil { + return 0, errors.Wrapf(err, "invalid interval format: %s", durationStr) + } + unit := durationStr[endIndex] + + switch unit { + case 'd': + totalMinutes += value * 24 * 60 + case 'h': + totalMinutes += value * 60 + case 'm': + totalMinutes += value + default: + return 0, errors.Errorf("invalid interval unit: %s", string(unit)) + } + + i = endIndex + 1 + } + + return time.Duration(totalMinutes) * time.Minute, nil +} From 5da1f9eb94f493d7a386924c5264b798db01bba3 Mon Sep 17 00:00:00 2001 From: Parthiba-Hazra Date: Sun, 18 Feb 2024 22:25:23 +0530 Subject: [PATCH 02/11] add the pruning logic,for now we keep it 7 days as default pruning threshold and fix some previous functionalities Signed-off-by: Parthiba-Hazra --- Makefile | 2 +- pkg/image/fetcher.go | 27 ++++------ pkg/image/pull_policy.go | 105 ++++++++++++++++++++++++++++++++++++--- 3 files changed, 109 insertions(+), 25 deletions(-) diff --git a/Makefile b/Makefile index f365fbcdff..2009dc6a47 100644 --- a/Makefile +++ b/Makefile @@ -154,7 +154,7 @@ install: mkdir -p $(IMAGE_JSON_DIR); \ fi @if [ ! -f "$(IMAGE_JSON_FILE)" ]; then \ - echo '{ "interval": { "duration": "" }, "image": {} }' > $(IMAGE_JSON_FILE); \ + echo '{ "interval": { "pulling_interval": "", pruning_interval: "7d", "last_prune": "" }, "image": {} }' > $(IMAGE_JSON_FILE); \ fi ## install-mockgen: Used only by apt-get install when installing ubuntu ppa diff --git a/pkg/image/fetcher.go b/pkg/image/fetcher.go index ce813d8ace..4458f32195 100644 --- a/pkg/image/fetcher.go +++ b/pkg/image/fetcher.go @@ -37,15 +37,6 @@ type LayoutOption struct { Sparse bool } -type ImageJSON struct { - Interval struct { - Duration string `json:"duration"` - } `json:"interval"` - Image struct { - ImageIDtoTIME map[string]string - } `json:"image"` -} - // WithRegistryMirrors supply your own mirrors for registry. func WithRegistryMirrors(registryMirrors map[string]string) FetcherOption { return func(c *Fetcher) { @@ -126,14 +117,18 @@ func (f *Fetcher) Fetch(ctx context.Context, name string, options FetchOptions) return img, err } case PullWithInterval: - pull, err := CheckImagePullInterval(imageID) - if !pull { - if err != nil { - return nil, err - } - img, err := f.fetchDaemonImage(name) + pull, err := f.CheckImagePullInterval(imageID) + if err != nil { + return nil, err + } + img, err := f.fetchDaemonImage(name) + if !pull && err != nil && !errors.Is(err, ErrNotFound) { return img, err } + err = f.PruneOldImages() + if err != nil { + f.logger.Warnf("Failed to prune images that are older than 7 days, %s", err) + } } f.logger.Debugf("Pulling image %s", style.Symbol(name)) @@ -286,7 +281,7 @@ func (w *colorizedWriter) Write(p []byte) (n int, err error) { } func (f *Fetcher) updateImagePullRecord(imageID, timestamp string) error { - imageJSON, err := readImageJSON() + imageJSON, err := readImageJSON(f.logger) if err != nil { return err } diff --git a/pkg/image/pull_policy.go b/pkg/image/pull_policy.go index 13af83cc92..d9fe202f57 100644 --- a/pkg/image/pull_policy.go +++ b/pkg/image/pull_policy.go @@ -10,13 +10,14 @@ import ( "strings" "time" + "github.com/buildpacks/pack/pkg/logging" "github.com/pkg/errors" ) // PullPolicy defines a policy for how to manage images type PullPolicy int -var interval time.Duration +var interval string var ( intervalRegex = regexp.MustCompile(`^(\d+d)?(\d+h)?(\d+m)?$`) @@ -34,6 +35,17 @@ const ( PullWithInterval ) +type ImageJSON struct { + Interval struct { + PullingInterval string `json:"pulling_interval"` + PruningIinterval string `json:"pruning_interval"` + LastPrune string `json:"last_prune"` + } `json:"interval"` + Image struct { + ImageIDtoTIME map[string]string + } `json:"image"` +} + var nameMap = map[string]PullPolicy{"always": PullAlways, "never": PullNever, "if-not-present": PullIfNotPresent, "": PullAlways} // ParsePullPolicy from string with support for interval formats @@ -43,6 +55,7 @@ func ParsePullPolicy(policy string) (PullPolicy, error) { } if strings.HasPrefix(policy, "interval=") { + interval = policy intervalStr := strings.TrimPrefix(policy, "interval=") matches := intervalRegex.FindStringSubmatch(intervalStr) if len(matches) == 0 { @@ -73,12 +86,12 @@ func (p PullPolicy) String() string { } func updateImageJSONDuration(intervalStr string) error { - imageJSON, err := readImageJSON() + imageJSON, err := readImageJSON(logging.NewSimpleLogger(os.Stderr)) if err != nil { return err } - imageJSON.Interval.Duration = intervalStr + imageJSON.Interval.PullingInterval = intervalStr updatedJSON, err := json.MarshalIndent(imageJSON, "", " ") if err != nil { @@ -88,13 +101,33 @@ func updateImageJSONDuration(intervalStr string) error { return os.WriteFile(imagePath, updatedJSON, 0644) } -func readImageJSON() (ImageJSON, error) { +func readImageJSON(l logging.Logger) (ImageJSON, error) { homeDir, err := os.UserHomeDir() if err != nil { return ImageJSON{}, errors.Wrap(err, "failed to get home directory") } imagePath = filepath.Join(homeDir, ".pack", "image.json") + // Check if the directory exists, if not, create it + dirPath := filepath.Dir(imagePath) + if _, err := os.Stat(dirPath); os.IsNotExist(err) { + l.Warnf("missing `.pack` directory under %s directory %s", homeDir, err) + l.Debugf("creating `.pack` directory under %s directory", homeDir) + if err := os.MkdirAll(dirPath, 0755); err != nil { + return ImageJSON{}, errors.Wrap(err, "failed to create directory") + } + } + + // Check if the file exists, if not, create it with minimum JSON + if _, err := os.Stat(imagePath); os.IsNotExist(err) { + l.Warnf("missing `image.json` file under %s directory %s", dirPath, err) + l.Debugf("creating `image.json` file under %s directory", dirPath) + minimumJSON := []byte(`{"interval":{"pulling_interval":"","pruning_interval":"7d","last_prune":""},"image":{}}`) + if err := os.WriteFile(imagePath, minimumJSON, 0644); err != nil { + return ImageJSON{}, errors.Wrap(err, "failed to create image.json file") + } + } + jsonData, err := os.ReadFile(imagePath) if err != nil && !os.IsNotExist(err) { return ImageJSON{}, errors.Wrap(err, "failed to read image.json") @@ -108,8 +141,8 @@ func readImageJSON() (ImageJSON, error) { return imageJSON, nil } -func CheckImagePullInterval(imageID string) (bool, error) { - imageJSON, err := readImageJSON() +func (f *Fetcher) CheckImagePullInterval(imageID string) (bool, error) { + imageJSON, err := readImageJSON(f.logger) if err != nil { return false, err } @@ -125,7 +158,7 @@ func CheckImagePullInterval(imageID string) (bool, error) { return false, errors.Wrap(err, "failed to parse image timestamp from JSON") } - durationStr := imageJSON.Interval.Duration + durationStr := imageJSON.Interval.PullingInterval duration, err := parseDurationString(durationStr) if err != nil { @@ -159,7 +192,7 @@ func parseDurationString(durationStr string) (time.Duration, error) { case 'm': totalMinutes += value default: - return 0, errors.Errorf("invalid interval unit: %s", string(unit)) + return 0, errors.Errorf("invalid interval uniit: %s", string(unit)) } i = endIndex + 1 @@ -167,3 +200,59 @@ func parseDurationString(durationStr string) (time.Duration, error) { return time.Duration(totalMinutes) * time.Minute, nil } + +func (f *Fetcher) PruneOldImages() error { + imageJSON, err := readImageJSON(f.logger) + if err != nil { + return err + } + + if imageJSON.Interval.LastPrune != "" { + lastPruneTime, err := time.Parse(time.RFC3339, imageJSON.Interval.LastPrune) + if err != nil { + return errors.Wrap(err, "failed to parse last prune timestamp from JSON") + } + + pruningInterval, err := parseDurationString(imageJSON.Interval.PruningIinterval) + if err != nil { + return errors.Wrap(err, "failed to parse pruning interval from JSON") + } + + if time.Since(lastPruneTime) < pruningInterval { + // not enough time has passed since the last prune + return nil + } + } + + // prune images older than the pruning interval + pruningDuration, err := parseDurationString(imageJSON.Interval.PruningIinterval) + if err != nil { + return errors.Wrap(err, "failed to parse pruning interval from JSON") + } + + pruningThreshold := time.Now().Add(-pruningDuration) + + for imageID, timestamp := range imageJSON.Image.ImageIDtoTIME { + imageTimestamp, err := time.Parse(time.RFC3339, timestamp) + if err != nil { + return errors.Wrap(err, "failed to parse image timestamp fron JSON") + } + + if imageTimestamp.Before(pruningThreshold) { + delete(imageJSON.Image.ImageIDtoTIME, imageID) + } + } + + imageJSON.Interval.LastPrune = time.Now().Format(time.RFC3339) + + updatedJSON, err := json.MarshalIndent(imageJSON, "", " ") + if err != nil { + return errors.Wrap(err, "failed to marshal updated records") + } + + if err := os.WriteFile(imagePath, updatedJSON, 0644); err != nil { + return errors.Wrap(err, "failed to write updated image.json") + } + + return nil +} From 30e299f2c78284b07ac198377c7a68deec8df6f6 Mon Sep 17 00:00:00 2001 From: Parthiba-Hazra Date: Tue, 20 Feb 2024 22:38:37 +0530 Subject: [PATCH 03/11] add support for setting prune interval and fix some logic in pull-policy's functionalities Signed-off-by: Parthiba-Hazra some minor changes Signed-off-by: Parthiba-Hazra minor changes Signed-off-by: Parthiba-Hazra --- Makefile | 2 +- internal/commands/config.go | 1 + internal/commands/config_prune_interval.go | 104 +++++++++++++++++++++ internal/commands/config_pull_policy.go | 4 +- pkg/image/fetcher.go | 37 ++++---- pkg/image/pull_policy.go | 78 +++++++++++++--- 6 files changed, 192 insertions(+), 34 deletions(-) create mode 100644 internal/commands/config_prune_interval.go diff --git a/Makefile b/Makefile index 2009dc6a47..97c89ea2c4 100644 --- a/Makefile +++ b/Makefile @@ -154,7 +154,7 @@ install: mkdir -p $(IMAGE_JSON_DIR); \ fi @if [ ! -f "$(IMAGE_JSON_FILE)" ]; then \ - echo '{ "interval": { "pulling_interval": "", pruning_interval: "7d", "last_prune": "" }, "image": {} }' > $(IMAGE_JSON_FILE); \ + echo '{ "interval": { "pulling_interval": "", "pruning_interval": "7d", "last_prune": "" }, "image": {} }' > $(IMAGE_JSON_FILE); \ fi ## install-mockgen: Used only by apt-get install when installing ubuntu ppa diff --git a/internal/commands/config.go b/internal/commands/config.go index a2b087dabc..0d183bc9be 100644 --- a/internal/commands/config.go +++ b/internal/commands/config.go @@ -19,6 +19,7 @@ func NewConfigCommand(logger logging.Logger, cfg config.Config, cfgPath string, cmd.AddCommand(ConfigDefaultBuilder(logger, cfg, cfgPath, client)) cmd.AddCommand(ConfigExperimental(logger, cfg, cfgPath)) cmd.AddCommand(ConfigPullPolicy(logger, cfg, cfgPath)) + cmd.AddCommand(ConfigPruneInterval(logger, cfg, cfgPath)) cmd.AddCommand(ConfigRegistries(logger, cfg, cfgPath)) cmd.AddCommand(ConfigRunImagesMirrors(logger, cfg, cfgPath)) cmd.AddCommand(ConfigTrustedBuilder(logger, cfg, cfgPath)) diff --git a/internal/commands/config_prune_interval.go b/internal/commands/config_prune_interval.go new file mode 100644 index 0000000000..a1926e3ea3 --- /dev/null +++ b/internal/commands/config_prune_interval.go @@ -0,0 +1,104 @@ +package commands + +import ( + "encoding/json" + "fmt" + "regexp" + + "github.com/buildpacks/pack/internal/config" + "github.com/buildpacks/pack/internal/style" + "github.com/buildpacks/pack/pkg/image" + "github.com/buildpacks/pack/pkg/logging" + "github.com/pkg/errors" + "github.com/spf13/cobra" +) + +func ConfigPruneInterval(logger logging.Logger, cfg config.Config, cfgPath string) *cobra.Command { + var unset bool + var intervalRegex = regexp.MustCompile(`^(\d+d)?(\d+h)?(\d+m)?$`) + + cmd := &cobra.Command{ + Use: "prune-interval", + Args: cobra.MaximumNArgs(1), + Short: "List, set, and unset the global pruning interval used for cleaning up outdated image entries from $HOME/.pack/image.json file", + Long: "You can use this command to list, set, and unset the default pruning interval for cleaning up unused images:\n" + + "* To list your current pruning interval, run `pack config prune-interval`.\n" + + "* To set a new pruning interval, run `pack config prune-interval ` where is a duration string (e.g., '7d' for 7 days).\n" + + "* To unset the pruning interval, run `pack config prune-interval --unset`.\n" + + fmt.Sprintf("Unsetting the pruning interval will reset the interval to the default, which is %s.", style.Symbol("7 days")), + RunE: logError(logger, func(cmd *cobra.Command, args []string) error { + switch { + case unset: + if len(args) > 0 { + return errors.Errorf("prune inteval and --unset cannot be specified simultaneously") + } + imageJSON, err := image.ReadImageJSON(logger) + if err != nil { + return err + } + oldPruneInterval := imageJSON.Interval.PruningIinterval + imageJSON.Interval.PruningIinterval = "7d" + + updatedJSON, err := json.MarshalIndent(imageJSON, "", " ") + if err != nil { + return errors.Wrap(err, "failed to marshal updated records") + } + err = image.WriteFile(updatedJSON) + if err != nil { + return err + } + logger.Infof("Successfully unset pruning interval %s", style.Symbol(oldPruneInterval)) + logger.Infof("Pruning interval has been set to %s", style.Symbol(imageJSON.Interval.PruningIinterval)) + case len(args) == 0: // list + imageJSON, err := image.ReadImageJSON(logger) + if err != nil { + return err + } + pruneInterval := imageJSON.Interval.PruningIinterval + if err != nil { + return err + } + + logger.Infof("The current prune interval is %s", style.Symbol(pruneInterval)) + default: // set + newPruneInterval := args[0] + + imageJSON, err := image.ReadImageJSON(logger) + if err != nil { + return err + } + pruneInterval := imageJSON.Interval.PruningIinterval + if err != nil { + return err + } + + if newPruneInterval == pruneInterval { + logger.Infof("Prune Interval is already set to %s", style.Symbol(newPruneInterval)) + return nil + } + + matches := intervalRegex.FindStringSubmatch(newPruneInterval) + if len(matches) == 0 { + return errors.Errorf("invalid interval format: %s", newPruneInterval) + } + + imageJSON.Interval.PruningIinterval = newPruneInterval + updatedJSON, err := json.MarshalIndent(imageJSON, "", " ") + if err != nil { + return errors.Wrap(err, "failed to marshal updated records") + } + err = image.WriteFile(updatedJSON) + if err != nil { + return err + } + + logger.Infof("Successfully set %s as the pruning interval", style.Symbol(imageJSON.Interval.PruningIinterval)) + } + + return nil + }), + } + cmd.Flags().BoolVarP(&unset, "unset", "u", false, "Unset prune interval, and set it back to the default prune-interval, which is "+style.Symbol("7d")) + AddHelpFlag(cmd, "prune-interval") + return cmd +} diff --git a/internal/commands/config_pull_policy.go b/internal/commands/config_pull_policy.go index ebeac05e67..54711b26d6 100644 --- a/internal/commands/config_pull_policy.go +++ b/internal/commands/config_pull_policy.go @@ -16,12 +16,12 @@ func ConfigPullPolicy(logger logging.Logger, cfg config.Config, cfgPath string) var unset bool cmd := &cobra.Command{ - Use: "pull-policy ", + Use: "pull-policy | never>", Args: cobra.MaximumNArgs(1), Short: "List, set and unset the global pull policy used by other commands", Long: "You can use this command to list, set, and unset the default pull policy that will be used when working with containers:\n" + "* To list your pull policy, run `pack config pull-policy`.\n" + - "* To set your pull policy, run `pack config pull-policy `.\n" + + "* To set your pull policy, run `pack config pull-policy | never>`.\n" + "* To unset your pull policy, run `pack config pull-policy --unset`.\n" + fmt.Sprintf("Unsetting the pull policy will reset the policy to the default, which is %s", style.Symbol("always")), RunE: logError(logger, func(cmd *cobra.Command, args []string) error { diff --git a/pkg/image/fetcher.go b/pkg/image/fetcher.go index 4458f32195..f125ec7bab 100644 --- a/pkg/image/fetcher.go +++ b/pkg/image/fetcher.go @@ -5,7 +5,6 @@ import ( "encoding/base64" "encoding/json" "io" - "os" "strings" "time" @@ -91,14 +90,6 @@ func (f *Fetcher) Fetch(ctx context.Context, name string, options FetchOptions) return nil, err } - var imageID string // Variable to store the image ID - parts := strings.Split(name, "@") - if len(parts) > 1 { - imageID = parts[1] - } else { - imageID = parts[0] - } - if (options.LayoutOption != LayoutOption{}) { return f.fetchLayoutImage(name, options.LayoutOption) } @@ -116,15 +107,18 @@ func (f *Fetcher) Fetch(ctx context.Context, name string, options FetchOptions) if err == nil || !errors.Is(err, ErrNotFound) { return img, err } - case PullWithInterval: - pull, err := f.CheckImagePullInterval(imageID) - if err != nil { - return nil, err - } + case PullWithInterval, PullDaily, PullHourly, PullWeekly: img, err := f.fetchDaemonImage(name) - if !pull && err != nil && !errors.Is(err, ErrNotFound) { + if err != nil && !errors.Is(err, ErrNotFound) { return img, err } + if err == nil { + pull, err := f.CheckImagePullInterval(name) + if !pull { + return img, err + } + } + err = f.PruneOldImages() if err != nil { f.logger.Warnf("Failed to prune images that are older than 7 days, %s", err) @@ -149,7 +143,7 @@ func (f *Fetcher) Fetch(ctx context.Context, name string, options FetchOptions) } // Update image pull record in the JSON file - if err := f.updateImagePullRecord(imageID, time.Now().Format(time.RFC3339)); err != nil { + if err := f.updateImagePullRecord(name, time.Now().Format(time.RFC3339)); err != nil { return nil, err } @@ -281,7 +275,7 @@ func (w *colorizedWriter) Write(p []byte) (n int, err error) { } func (f *Fetcher) updateImagePullRecord(imageID, timestamp string) error { - imageJSON, err := readImageJSON(f.logger) + imageJSON, err := ReadImageJSON(f.logger) if err != nil { return err } @@ -293,8 +287,13 @@ func (f *Fetcher) updateImagePullRecord(imageID, timestamp string) error { updatedJSON, err := json.MarshalIndent(imageJSON, "", " ") if err != nil { - return errors.Wrap(err, "failed to marshal updated records") + return errors.New("failed to marshal updated records: " + err.Error()) + } + + err = WriteFile(updatedJSON) + if err != nil { + return err } - return os.WriteFile(imagePath, updatedJSON, 0644) + return nil } diff --git a/pkg/image/pull_policy.go b/pkg/image/pull_policy.go index d9fe202f57..a3601a0251 100644 --- a/pkg/image/pull_policy.go +++ b/pkg/image/pull_policy.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" "os" + "os/exec" "path/filepath" "regexp" "strconv" @@ -20,14 +21,23 @@ type PullPolicy int var interval string var ( + hourly = "1h" + daily = "1d" + weekly = "7d" intervalRegex = regexp.MustCompile(`^(\d+d)?(\d+h)?(\d+m)?$`) imagePath string ) const ( - // PullAlways images, even if they are present + // Always pull images, even if they are present PullAlways PullPolicy = iota - // PullNever images, even if they are not present + // Pull images hourly + PullHourly + // Pull images daily + PullDaily + // Pull images weekly + PullWeekly + // Never pull images, even if they are not present PullNever // PullIfNotPresent pulls images if they aren't present PullIfNotPresent @@ -46,11 +56,29 @@ type ImageJSON struct { } `json:"image"` } -var nameMap = map[string]PullPolicy{"always": PullAlways, "never": PullNever, "if-not-present": PullIfNotPresent, "": PullAlways} +var nameMap = map[string]PullPolicy{"always": PullAlways, "hourly": PullHourly, "daily": PullDaily, "weekly": PullWeekly, "never": PullNever, "if-not-present": PullIfNotPresent, "": PullAlways} // ParsePullPolicy from string with support for interval formats func ParsePullPolicy(policy string) (PullPolicy, error) { if val, ok := nameMap[policy]; ok { + if val == PullHourly { + err := updateImageJSONDuration(hourly) + if err != nil { + return PullAlways, err + } + } + if val == PullDaily { + err := updateImageJSONDuration(daily) + if err != nil { + return PullAlways, err + } + } + if val == PullWeekly { + err := updateImageJSONDuration(weekly) + if err != nil { + return PullAlways, err + } + } return val, nil } @@ -62,7 +90,10 @@ func ParsePullPolicy(policy string) (PullPolicy, error) { return PullAlways, errors.Errorf("invalid interval format: %s", intervalStr) } - updateImageJSONDuration(intervalStr) + err := updateImageJSONDuration(intervalStr) + if err != nil { + return PullAlways, err + } return PullWithInterval, nil } @@ -74,19 +105,25 @@ func (p PullPolicy) String() string { switch p { case PullAlways: return "always" + case PullHourly: + return "hourly" + case PullDaily: + return "daily" + case PullWeekly: + return "weekly" case PullNever: return "never" case PullIfNotPresent: return "if-not-present" case PullWithInterval: - return fmt.Sprintf("interval=%v", interval) + return fmt.Sprintf("%v", interval) } return "" } func updateImageJSONDuration(intervalStr string) error { - imageJSON, err := readImageJSON(logging.NewSimpleLogger(os.Stderr)) + imageJSON, err := ReadImageJSON(logging.NewSimpleLogger(os.Stderr)) if err != nil { return err } @@ -98,10 +135,10 @@ func updateImageJSONDuration(intervalStr string) error { return errors.Wrap(err, "failed to marshal updated records") } - return os.WriteFile(imagePath, updatedJSON, 0644) + return WriteFile(updatedJSON) } -func readImageJSON(l logging.Logger) (ImageJSON, error) { +func ReadImageJSON(l logging.Logger) (ImageJSON, error) { homeDir, err := os.UserHomeDir() if err != nil { return ImageJSON{}, errors.Wrap(err, "failed to get home directory") @@ -123,7 +160,7 @@ func readImageJSON(l logging.Logger) (ImageJSON, error) { l.Warnf("missing `image.json` file under %s directory %s", dirPath, err) l.Debugf("creating `image.json` file under %s directory", dirPath) minimumJSON := []byte(`{"interval":{"pulling_interval":"","pruning_interval":"7d","last_prune":""},"image":{}}`) - if err := os.WriteFile(imagePath, minimumJSON, 0644); err != nil { + if err := WriteFile(minimumJSON); err != nil { return ImageJSON{}, errors.Wrap(err, "failed to create image.json file") } } @@ -142,7 +179,7 @@ func readImageJSON(l logging.Logger) (ImageJSON, error) { } func (f *Fetcher) CheckImagePullInterval(imageID string) (bool, error) { - imageJSON, err := readImageJSON(f.logger) + imageJSON, err := ReadImageJSON(f.logger) if err != nil { return false, err } @@ -202,7 +239,7 @@ func parseDurationString(durationStr string) (time.Duration, error) { } func (f *Fetcher) PruneOldImages() error { - imageJSON, err := readImageJSON(f.logger) + imageJSON, err := ReadImageJSON(f.logger) if err != nil { return err } @@ -238,6 +275,11 @@ func (f *Fetcher) PruneOldImages() error { return errors.Wrap(err, "failed to parse image timestamp fron JSON") } + _, err = f.fetchDaemonImage(imageID) + if !errors.Is(err, ErrNotFound) { + delete(imageJSON.Image.ImageIDtoTIME, imageID) + } + if imageTimestamp.Before(pruningThreshold) { delete(imageJSON.Image.ImageIDtoTIME, imageID) } @@ -250,9 +292,21 @@ func (f *Fetcher) PruneOldImages() error { return errors.Wrap(err, "failed to marshal updated records") } - if err := os.WriteFile(imagePath, updatedJSON, 0644); err != nil { + if err := WriteFile(updatedJSON); err != nil { return errors.Wrap(err, "failed to write updated image.json") } return nil } + +func WriteFile(data []byte) error { + cmd := exec.Command("sudo", "sh", "-c", "echo '"+string(data)+"' > "+imagePath) + cmd.Stderr = os.Stderr + cmd.Stdin = os.Stdin + + if err := cmd.Run(); err != nil { + return errors.New("failed to write file with sudo: " + err.Error()) + } + + return nil +} From 23f2da9ce2e7b466f0d63d83beabf40cde1491a6 Mon Sep 17 00:00:00 2001 From: Parthiba-Hazra Date: Wed, 28 Feb 2024 13:15:17 +0530 Subject: [PATCH 04/11] refactor: update the interval case to check the interval first before deciding wheather to fetch the image from daemon or not Signed-off-by: Parthiba-Hazra --- pkg/image/fetcher.go | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/pkg/image/fetcher.go b/pkg/image/fetcher.go index f125ec7bab..f4f29cbccb 100644 --- a/pkg/image/fetcher.go +++ b/pkg/image/fetcher.go @@ -108,20 +108,22 @@ func (f *Fetcher) Fetch(ctx context.Context, name string, options FetchOptions) return img, err } case PullWithInterval, PullDaily, PullHourly, PullWeekly: - img, err := f.fetchDaemonImage(name) - if err != nil && !errors.Is(err, ErrNotFound) { - return img, err + pull, err := f.CheckImagePullInterval(name) + if err != nil { + f.logger.Warnf("failed to check pulling interval for image %s, %s", name, err) } - if err == nil { - pull, err := f.CheckImagePullInterval(name) - if !pull { - return img, err + if !pull { + img, err := f.fetchDaemonImage(name) + if errors.Is(err, ErrNotFound) { + imageJSON, _ := ReadImageJSON(f.logger) + delete(imageJSON.Image.ImageIDtoTIME, name) } + return img, err } err = f.PruneOldImages() if err != nil { - f.logger.Warnf("Failed to prune images that are older than 7 days, %s", err) + f.logger.Warnf("Failed to prune images, %s", err) } } From 213f04b4bc6bff7e2d0d26ba42fef1510ff7e4e5 Mon Sep 17 00:00:00 2001 From: Parthiba-Hazra Date: Sun, 3 Mar 2024 23:48:02 +0530 Subject: [PATCH 05/11] Test: Add test cases for newly added pull-policies - add test cases for new pull-policies(PullWithInterval, PullHourly PullDaily, PullWeekly) Signed-off-by: Parthiba-Hazra --- Makefile | 10 - internal/commands/config_prune_interval.go | 14 +- internal/commands/config_pull_policy_test.go | 48 ++++ pkg/client/client.go | 3 +- pkg/image/fetcher.go | 88 +++++- pkg/image/fetcher_test.go | 273 ++++++++++++++++++- pkg/image/pull_policy.go | 67 ++--- pkg/image/pull_policy_test.go | 33 ++- 8 files changed, 454 insertions(+), 82 deletions(-) diff --git a/Makefile b/Makefile index 97c89ea2c4..0d1246ea27 100644 --- a/Makefile +++ b/Makefile @@ -33,10 +33,6 @@ TEST_TIMEOUT?=1200s UNIT_TIMEOUT?=$(TEST_TIMEOUT) NO_DOCKER?= -# Define the directory for the image.json file -IMAGE_JSON_DIR := $(HOME)/.pack -IMAGE_JSON_FILE := $(IMAGE_JSON_DIR)/image.json - # Clean build flags clean_build := $(strip ${PACK_BUILD}) clean_sha := $(strip ${PACK_GITSHA1}) @@ -150,12 +146,6 @@ package: out install: mkdir -p ${DESTDIR}${BINDIR} cp ./out/$(PACK_BIN) ${DESTDIR}${BINDIR}/ - @if [ ! -d "$(IMAGE_JSON_DIR)" ]; then \ - mkdir -p $(IMAGE_JSON_DIR); \ - fi - @if [ ! -f "$(IMAGE_JSON_FILE)" ]; then \ - echo '{ "interval": { "pulling_interval": "", "pruning_interval": "7d", "last_prune": "" }, "image": {} }' > $(IMAGE_JSON_FILE); \ - fi ## install-mockgen: Used only by apt-get install when installing ubuntu ppa install-mockgen: diff --git a/internal/commands/config_prune_interval.go b/internal/commands/config_prune_interval.go index a1926e3ea3..9e1d8513ad 100644 --- a/internal/commands/config_prune_interval.go +++ b/internal/commands/config_prune_interval.go @@ -36,8 +36,8 @@ func ConfigPruneInterval(logger logging.Logger, cfg config.Config, cfgPath strin if err != nil { return err } - oldPruneInterval := imageJSON.Interval.PruningIinterval - imageJSON.Interval.PruningIinterval = "7d" + oldPruneInterval := imageJSON.Interval.PruningInterval + imageJSON.Interval.PruningInterval = "7d" updatedJSON, err := json.MarshalIndent(imageJSON, "", " ") if err != nil { @@ -48,13 +48,13 @@ func ConfigPruneInterval(logger logging.Logger, cfg config.Config, cfgPath strin return err } logger.Infof("Successfully unset pruning interval %s", style.Symbol(oldPruneInterval)) - logger.Infof("Pruning interval has been set to %s", style.Symbol(imageJSON.Interval.PruningIinterval)) + logger.Infof("Pruning interval has been set to %s", style.Symbol(imageJSON.Interval.PruningInterval)) case len(args) == 0: // list imageJSON, err := image.ReadImageJSON(logger) if err != nil { return err } - pruneInterval := imageJSON.Interval.PruningIinterval + pruneInterval := imageJSON.Interval.PruningInterval if err != nil { return err } @@ -67,7 +67,7 @@ func ConfigPruneInterval(logger logging.Logger, cfg config.Config, cfgPath strin if err != nil { return err } - pruneInterval := imageJSON.Interval.PruningIinterval + pruneInterval := imageJSON.Interval.PruningInterval if err != nil { return err } @@ -82,7 +82,7 @@ func ConfigPruneInterval(logger logging.Logger, cfg config.Config, cfgPath strin return errors.Errorf("invalid interval format: %s", newPruneInterval) } - imageJSON.Interval.PruningIinterval = newPruneInterval + imageJSON.Interval.PruningInterval = newPruneInterval updatedJSON, err := json.MarshalIndent(imageJSON, "", " ") if err != nil { return errors.Wrap(err, "failed to marshal updated records") @@ -92,7 +92,7 @@ func ConfigPruneInterval(logger logging.Logger, cfg config.Config, cfgPath strin return err } - logger.Infof("Successfully set %s as the pruning interval", style.Symbol(imageJSON.Interval.PruningIinterval)) + logger.Infof("Successfully set %s as the pruning interval", style.Symbol(imageJSON.Interval.PruningInterval)) } return nil diff --git a/internal/commands/config_pull_policy_test.go b/internal/commands/config_pull_policy_test.go index 7674ab2ab9..37b9ecfc01 100644 --- a/internal/commands/config_pull_policy_test.go +++ b/internal/commands/config_pull_policy_test.go @@ -96,6 +96,54 @@ func testConfigPullPolicyCommand(t *testing.T, when spec.G, it spec.S) { assert.Contains(outBuf.String(), "if-not-present") }) }) + + when("policy set to hourly in config", func() { + it("lists hourly as pull policy", func() { + cfg.PullPolicy = "hourly" + command = commands.ConfigPullPolicy(logger, cfg, configFile) + command.SetArgs([]string{}) + + h.AssertNil(t, command.Execute()) + + assert.Contains(outBuf.String(), "hourly") + }) + }) + + when("policy set to daily in config", func() { + it("lists daily as pull policy", func() { + cfg.PullPolicy = "daily" + command = commands.ConfigPullPolicy(logger, cfg, configFile) + command.SetArgs([]string{}) + + h.AssertNil(t, command.Execute()) + + assert.Contains(outBuf.String(), "daily") + }) + }) + + when("policy set to weekly in config", func() { + it("lists weekly as pull policy", func() { + cfg.PullPolicy = "weekly" + command = commands.ConfigPullPolicy(logger, cfg, configFile) + command.SetArgs([]string{}) + + h.AssertNil(t, command.Execute()) + + assert.Contains(outBuf.String(), "weekly") + }) + }) + + when("policy set to interval=1d2h30m in config", func() { + it("lists interval=1d2h30m as pull policy", func() { + cfg.PullPolicy = "interval=1d2h30m" + command = commands.ConfigPullPolicy(logger, cfg, configFile) + command.SetArgs([]string{}) + + h.AssertNil(t, command.Execute()) + + assert.Contains(outBuf.String(), "interval=1d2h30m") + }) + }) }) when("set", func() { when("policy provided is the same as configured pull policy", func() { diff --git a/pkg/client/client.go b/pkg/client/client.go index d034851fc6..9786b61e0b 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -231,7 +231,8 @@ func NewClient(opts ...Option) (*Client, error) { } if client.imageFetcher == nil { - client.imageFetcher = image.NewFetcher(client.logger, client.docker, image.WithRegistryMirrors(client.registryMirrors), image.WithKeychain(client.keychain)) + imagePullChecker := image.NewPullChecker(client.logger) + client.imageFetcher = image.NewFetcher(client.logger, client.docker, imagePullChecker, image.WithRegistryMirrors(client.registryMirrors), image.WithKeychain(client.keychain)) } if client.imageFactory == nil { diff --git a/pkg/image/fetcher.go b/pkg/image/fetcher.go index f4f29cbccb..247e88a4cd 100644 --- a/pkg/image/fetcher.go +++ b/pkg/image/fetcher.go @@ -36,6 +36,19 @@ type LayoutOption struct { Sparse bool } +type ImagePullChecker interface { + CheckImagePullInterval(imageID string, l logging.Logger) (bool, error) + ReadImageJSON(l logging.Logger) (*ImageJSON, error) +} + +type PullChecker struct { + logger logging.Logger +} + +func NewPullChecker(logger logging.Logger) *PullChecker { + return &PullChecker{logger: logger} +} + // WithRegistryMirrors supply your own mirrors for registry. func WithRegistryMirrors(registryMirrors map[string]string) FetcherOption { return func(c *Fetcher) { @@ -55,10 +68,11 @@ type DockerClient interface { } type Fetcher struct { - docker DockerClient - logger logging.Logger - registryMirrors map[string]string - keychain authn.Keychain + docker DockerClient + logger logging.Logger + registryMirrors map[string]string + keychain authn.Keychain + imagePullChecker ImagePullChecker } type FetchOptions struct { @@ -68,11 +82,12 @@ type FetchOptions struct { LayoutOption LayoutOption } -func NewFetcher(logger logging.Logger, docker DockerClient, opts ...FetcherOption) *Fetcher { +func NewFetcher(logger logging.Logger, docker DockerClient, imagePullChecker ImagePullChecker, opts ...FetcherOption) *Fetcher { fetcher := &Fetcher{ - logger: logger, - docker: docker, - keychain: authn.DefaultKeychain, + logger: logger, + docker: docker, + keychain: authn.DefaultKeychain, + imagePullChecker: imagePullChecker, } for _, opt := range opts { @@ -108,15 +123,23 @@ func (f *Fetcher) Fetch(ctx context.Context, name string, options FetchOptions) return img, err } case PullWithInterval, PullDaily, PullHourly, PullWeekly: - pull, err := f.CheckImagePullInterval(name) + pull, err := f.imagePullChecker.CheckImagePullInterval(name, f.logger) if err != nil { f.logger.Warnf("failed to check pulling interval for image %s, %s", name, err) } if !pull { img, err := f.fetchDaemonImage(name) if errors.Is(err, ErrNotFound) { - imageJSON, _ := ReadImageJSON(f.logger) + imageJSON, _ := f.imagePullChecker.ReadImageJSON(f.logger) delete(imageJSON.Image.ImageIDtoTIME, name) + updatedJSON, err := json.MarshalIndent(imageJSON, "", " ") + if err != nil { + f.logger.Errorf("failed to marshal updated records %s", err) + } + + if err := WriteFile(updatedJSON); err != nil { + f.logger.Errorf("failed to write updated image.json %s", err) + } } return img, err } @@ -144,9 +167,11 @@ func (f *Fetcher) Fetch(ctx context.Context, name string, options FetchOptions) return nil, err } - // Update image pull record in the JSON file - if err := f.updateImagePullRecord(name, time.Now().Format(time.RFC3339)); err != nil { - return nil, err + if options.PullPolicy == PullWithInterval || options.PullPolicy == PullHourly || options.PullPolicy == PullDaily || options.PullPolicy == PullWeekly { + // Update image pull record in the JSON file + if err := f.updateImagePullRecord(name, time.Now().Format(time.RFC3339)); err != nil { + return nil, err + } } return image, nil @@ -299,3 +324,40 @@ func (f *Fetcher) updateImagePullRecord(imageID, timestamp string) error { return nil } + +func (c *PullChecker) CheckImagePullInterval(imageID string, l logging.Logger) (bool, error) { + return CheckImagePullInterval(imageID, c.logger) +} + +func (c *PullChecker) ReadImageJSON(l logging.Logger) (*ImageJSON, error) { + return ReadImageJSON(c.logger) +} + +func CheckImagePullInterval(imageID string, l logging.Logger) (bool, error) { + imageJSON, err := ReadImageJSON(l) + if err != nil { + return false, err + } + + timestamp, ok := imageJSON.Image.ImageIDtoTIME[imageID] + if !ok { + // If the image ID is not present, return true + return true, nil + } + + imageTimestamp, err := time.Parse(time.RFC3339, timestamp) + if err != nil { + return false, errors.Wrap(err, "failed to parse image timestamp from JSON") + } + + durationStr := imageJSON.Interval.PullingInterval + + duration, err := parseDurationString(durationStr) + if err != nil { + return false, errors.Wrap(err, "failed to parse duration from JSON") + } + + timeThreshold := time.Now().Add(-duration) + + return imageTimestamp.Before(timeThreshold), nil +} diff --git a/pkg/image/fetcher_test.go b/pkg/image/fetcher_test.go index 3b119e15f3..d1ea37e35c 100644 --- a/pkg/image/fetcher_test.go +++ b/pkg/image/fetcher_test.go @@ -25,7 +25,42 @@ import ( ) var docker client.CommonAPIClient +var logger logging.Logger var registryConfig *h.TestRegistryConfig +var imageJSON *image.ImageJSON +var mockImagePullChecker = NewMockImagePullChecker(logger) + +type MockPullChecker struct { + *image.PullChecker + MockCheckImagePullInterval func(imageID string, l logging.Logger) (bool, error) + MockReadImageJSON func(l logging.Logger) (*image.ImageJSON, error) +} + +func NewMockImagePullChecker(logger logging.Logger) *MockPullChecker { + return &MockPullChecker{ + PullChecker: image.NewPullChecker(logger), + } +} + +func (m *MockPullChecker) CheckImagePullInterval(imageID string, l logging.Logger) (bool, error) { + if m.MockCheckImagePullInterval != nil { + return m.MockCheckImagePullInterval(imageID, l) + } + return false, nil +} + +func (m *MockPullChecker) ReadImageJSON(l logging.Logger) (*image.ImageJSON, error) { + if m.MockReadImageJSON != nil { + return m.MockReadImageJSON(l) + } + return &image.ImageJSON{ + Image: &image.ImageData{ + ImageIDtoTIME: map[string]string{ + "repoName": "2023-01-01T00:00:00Z", + }, + }, + }, nil +} func TestFetcher(t *testing.T) { color.Disable(true) @@ -57,7 +92,8 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { it.Before(func() { repo = "some-org/" + h.RandString(10) repoName = registryConfig.RepoName(repo) - imageFetcher = image.NewFetcher(logging.NewLogWithWriters(&outBuf, &outBuf), docker) + logger = logging.NewLogWithWriters(&outBuf, &outBuf) + imageFetcher = image.NewFetcher(logger, docker, mockImagePullChecker) info, err := docker.Info(context.TODO()) h.AssertNil(t, err) @@ -168,7 +204,10 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { var outCons *color.Console outCons, output = h.MockWriterAndOutput() logger = logging.NewLogWithWriters(outCons, outCons) - imageFetcher = image.NewFetcher(logger, docker) + mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, l logging.Logger) (bool, error) { + return true, nil + } + imageFetcher = image.NewFetcher(logger, docker, mockImagePullChecker) }) it.After(func() { @@ -342,6 +381,236 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { }) }) }) + + when("PullWithInterval, PullHourly, PullDaily, PullWeekly", func() { + when("there is a remote image", func() { + var ( + label = "label" + remoteImgLabel string + ) + + it.Before(func() { + // Instantiate a pull-able local image + // as opposed to a remote image so that the image + // is created with the OS of the docker daemon + remoteImg, err := local.NewImage(repoName, docker) + h.AssertNil(t, err) + defer h.DockerRmi(docker, repoName) + + h.AssertNil(t, remoteImg.SetLabel(label, "1")) + h.AssertNil(t, remoteImg.Save()) + + h.AssertNil(t, h.PushImage(docker, remoteImg.Name(), registryConfig)) + + remoteImgLabel, err = remoteImg.Label(label) + h.AssertNil(t, err) + }) + + it.After(func() { + h.DockerRmi(docker, repoName) + }) + + when("there is no local image and CheckImagePullInterval returns true", func() { + it.Before(func() { + mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, l logging.Logger) (bool, error) { + return true, nil + } + imageFetcher = image.NewFetcher(logging.NewLogWithWriters(&outBuf, &outBuf), docker, mockImagePullChecker) + }) + + it.After(func() { + mockImagePullChecker.MockCheckImagePullInterval = nil + }) + + it("pulls the remote image and returns it", func() { + fetchedImg, err := imageFetcher.Fetch(context.TODO(), repoName, image.FetchOptions{Daemon: true, PullPolicy: image.PullWeekly}) + h.AssertNil(t, err) + + fetchedImgLabel, err := fetchedImg.Label(label) + h.AssertNil(t, err) + h.AssertEq(t, fetchedImgLabel, remoteImgLabel) + }) + }) + + when("there is no local image and CheckImagePullInterval returns false", func() { + it.Before(func() { + mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, l logging.Logger) (bool, error) { + return false, nil + } + imageFetcher = image.NewFetcher(logging.NewLogWithWriters(&outBuf, &outBuf), docker, mockImagePullChecker) + mockImagePullChecker.MockReadImageJSON = func(l logging.Logger) (*image.ImageJSON, error) { + return &image.ImageJSON{ + Image: &image.ImageData{ + ImageIDtoTIME: map[string]string{ + repoName: "2023-01-01T00:00:00Z", + }, + }, + }, nil + } + }) + + it.After(func() { + mockImagePullChecker.MockCheckImagePullInterval = nil + mockImagePullChecker.MockReadImageJSON = nil + }) + + it("returns an error and deletes the image record", func() { + _, err := imageFetcher.Fetch(context.TODO(), repoName, image.FetchOptions{Daemon: true, PullPolicy: image.PullWeekly}) + h.AssertError(t, err, fmt.Sprintf("image '%s' does not exist on the daemon", repoName)) + imageJSON, err = mockImagePullChecker.ReadImageJSON(logger) + h.AssertNil(t, err) + h.AssertNil(t, imageJSON.Image.ImageIDtoTIME) + }) + }) + + when("there is a local image and CheckImagePullInterval returns true", func() { + it.Before(func() { + img, err := local.NewImage(repoName, docker) + h.AssertNil(t, err) + + h.AssertNil(t, img.Save()) + mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, l logging.Logger) (bool, error) { + return true, nil + } + imageFetcher = image.NewFetcher(logging.NewLogWithWriters(&outBuf, &outBuf), docker, mockImagePullChecker) + }) + + it.After(func() { + mockImagePullChecker.MockCheckImagePullInterval = nil + h.DockerRmi(docker, repoName) + }) + + it("pulls the remote image and returns it", func() { + fetchedImg, err := imageFetcher.Fetch(context.TODO(), repoName, image.FetchOptions{Daemon: true, PullPolicy: image.PullIfNotPresent}) + h.AssertNil(t, err) + + fetchedImgLabel, err := fetchedImg.Label(label) + h.AssertNil(t, err) + h.AssertEq(t, fetchedImgLabel, remoteImgLabel) + }) + }) + + when("there is a local image and CheckImagePullInterval returns false", func() { + it.Before(func() { + img, err := local.NewImage(repoName, docker) + h.AssertNil(t, err) + + h.AssertNil(t, img.Save()) + mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, l logging.Logger) (bool, error) { + return true, nil + } + imageFetcher = image.NewFetcher(logging.NewLogWithWriters(&outBuf, &outBuf), docker, mockImagePullChecker) + }) + + it.After(func() { + mockImagePullChecker.MockCheckImagePullInterval = nil + h.DockerRmi(docker, repoName) + }) + + it("returns the local image", func() { + _, err := imageFetcher.Fetch(context.TODO(), repoName, image.FetchOptions{Daemon: true, PullPolicy: image.PullIfNotPresent}) + h.AssertNil(t, err) + }) + }) + }) + + when("there is no a remote image", func() { + when("there is no local image and CheckImagePullInterval returns true", func() { + it.Before(func() { + mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, l logging.Logger) (bool, error) { + return true, nil + } + imageFetcher = image.NewFetcher(logging.NewLogWithWriters(&outBuf, &outBuf), docker, mockImagePullChecker) + }) + + it.After(func() { + mockImagePullChecker.MockCheckImagePullInterval = nil + }) + + it("try to pull the remote image and returns error", func() { + _, err := imageFetcher.Fetch(context.TODO(), repoName, image.FetchOptions{Daemon: true, PullPolicy: image.PullWeekly}) + h.AssertNotNil(t, err) + }) + }) + + when("there is no local image and CheckImagePullInterval returns false", func() { + it.Before(func() { + mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, l logging.Logger) (bool, error) { + return false, nil + } + imageFetcher = image.NewFetcher(logging.NewLogWithWriters(&outBuf, &outBuf), docker, mockImagePullChecker) + mockImagePullChecker.MockReadImageJSON = func(l logging.Logger) (*image.ImageJSON, error) { + return &image.ImageJSON{ + Image: &image.ImageData{ + ImageIDtoTIME: map[string]string{ + repoName: "2023-01-01T00:00:00Z", + }, + }, + }, nil + } + }) + + it.After(func() { + mockImagePullChecker.MockCheckImagePullInterval = nil + mockImagePullChecker.MockReadImageJSON = nil + }) + + it("returns an error and deletes the image record", func() { + _, err := imageFetcher.Fetch(context.TODO(), repoName, image.FetchOptions{Daemon: true, PullPolicy: image.PullWeekly}) + h.AssertError(t, err, fmt.Sprintf("image '%s' does not exist on the daemon", repoName)) + imageJSON, err = mockImagePullChecker.ReadImageJSON(logger) + h.AssertNil(t, err) + h.AssertNil(t, imageJSON.Image.ImageIDtoTIME) + }) + }) + + when("there is a local image and CheckImagePullInterval returns true", func() { + it.Before(func() { + img, err := local.NewImage(repoName, docker) + h.AssertNil(t, err) + + h.AssertNil(t, img.Save()) + mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, l logging.Logger) (bool, error) { + return true, nil + } + imageFetcher = image.NewFetcher(logging.NewLogWithWriters(&outBuf, &outBuf), docker, mockImagePullChecker) + }) + + it.After(func() { + mockImagePullChecker.MockCheckImagePullInterval = nil + h.DockerRmi(docker, repoName) + }) + + it("try to pull the remote image and returns error", func() { + _, err := imageFetcher.Fetch(context.TODO(), repoName, image.FetchOptions{Daemon: true, PullPolicy: image.PullIfNotPresent}) + h.AssertNotNil(t, err) + }) + }) + + when("there is a local image and CheckImagePullInterval returns false", func() { + it.Before(func() { + img, err := local.NewImage(repoName, docker) + h.AssertNil(t, err) + + h.AssertNil(t, img.Save()) + mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, l logging.Logger) (bool, error) { + return true, nil + } + imageFetcher = image.NewFetcher(logging.NewLogWithWriters(&outBuf, &outBuf), docker, mockImagePullChecker) + }) + + it.After(func() { + mockImagePullChecker.MockCheckImagePullInterval = nil + h.DockerRmi(docker, repoName) + }) + + it("returns the local image", func() { + _, err := imageFetcher.Fetch(context.TODO(), repoName, image.FetchOptions{Daemon: true, PullPolicy: image.PullIfNotPresent}) + h.AssertNil(t, err) + }) + }) + }) + }) }) when("layout option is provided", func() { diff --git a/pkg/image/pull_policy.go b/pkg/image/pull_policy.go index a3601a0251..98c667f2ae 100644 --- a/pkg/image/pull_policy.go +++ b/pkg/image/pull_policy.go @@ -45,15 +45,19 @@ const ( PullWithInterval ) +type Interval struct { + PullingInterval string `json:"pulling_interval"` + PruningInterval string `json:"pruning_interval"` + LastPrune string `json:"last_prune"` +} + +type ImageData struct { + ImageIDtoTIME map[string]string +} + type ImageJSON struct { - Interval struct { - PullingInterval string `json:"pulling_interval"` - PruningIinterval string `json:"pruning_interval"` - LastPrune string `json:"last_prune"` - } `json:"interval"` - Image struct { - ImageIDtoTIME map[string]string - } `json:"image"` + Interval *Interval `json:"interval"` + Image *ImageData `json:"image"` } var nameMap = map[string]PullPolicy{"always": PullAlways, "hourly": PullHourly, "daily": PullDaily, "weekly": PullWeekly, "never": PullNever, "if-not-present": PullIfNotPresent, "": PullAlways} @@ -138,10 +142,10 @@ func updateImageJSONDuration(intervalStr string) error { return WriteFile(updatedJSON) } -func ReadImageJSON(l logging.Logger) (ImageJSON, error) { +func ReadImageJSON(l logging.Logger) (*ImageJSON, error) { homeDir, err := os.UserHomeDir() if err != nil { - return ImageJSON{}, errors.Wrap(err, "failed to get home directory") + return &ImageJSON{}, errors.Wrap(err, "failed to get home directory") } imagePath = filepath.Join(homeDir, ".pack", "image.json") @@ -151,7 +155,7 @@ func ReadImageJSON(l logging.Logger) (ImageJSON, error) { l.Warnf("missing `.pack` directory under %s directory %s", homeDir, err) l.Debugf("creating `.pack` directory under %s directory", homeDir) if err := os.MkdirAll(dirPath, 0755); err != nil { - return ImageJSON{}, errors.Wrap(err, "failed to create directory") + return &ImageJSON{}, errors.Wrap(err, "failed to create directory") } } @@ -161,52 +165,23 @@ func ReadImageJSON(l logging.Logger) (ImageJSON, error) { l.Debugf("creating `image.json` file under %s directory", dirPath) minimumJSON := []byte(`{"interval":{"pulling_interval":"","pruning_interval":"7d","last_prune":""},"image":{}}`) if err := WriteFile(minimumJSON); err != nil { - return ImageJSON{}, errors.Wrap(err, "failed to create image.json file") + return &ImageJSON{}, errors.Wrap(err, "failed to create image.json file") } } jsonData, err := os.ReadFile(imagePath) if err != nil && !os.IsNotExist(err) { - return ImageJSON{}, errors.Wrap(err, "failed to read image.json") + return &ImageJSON{}, errors.Wrap(err, "failed to read image.json") } - var imageJSON ImageJSON + var imageJSON *ImageJSON if err := json.Unmarshal(jsonData, &imageJSON); err != nil && !os.IsNotExist(err) { - return ImageJSON{}, errors.Wrap(err, "failed to unmarshal image.json") + return &ImageJSON{}, errors.Wrap(err, "failed to unmarshal image.json") } return imageJSON, nil } -func (f *Fetcher) CheckImagePullInterval(imageID string) (bool, error) { - imageJSON, err := ReadImageJSON(f.logger) - if err != nil { - return false, err - } - - timestamp, ok := imageJSON.Image.ImageIDtoTIME[imageID] - if !ok { - // If the image ID is not present, return true - return true, nil - } - - imageTimestamp, err := time.Parse(time.RFC3339, timestamp) - if err != nil { - return false, errors.Wrap(err, "failed to parse image timestamp from JSON") - } - - durationStr := imageJSON.Interval.PullingInterval - - duration, err := parseDurationString(durationStr) - if err != nil { - return false, errors.Wrap(err, "failed to parse duration from JSON") - } - - timeThreshold := time.Now().Add(-duration) - - return imageTimestamp.Before(timeThreshold), nil -} - func parseDurationString(durationStr string) (time.Duration, error) { var totalMinutes int for i := 0; i < len(durationStr); { @@ -250,7 +225,7 @@ func (f *Fetcher) PruneOldImages() error { return errors.Wrap(err, "failed to parse last prune timestamp from JSON") } - pruningInterval, err := parseDurationString(imageJSON.Interval.PruningIinterval) + pruningInterval, err := parseDurationString(imageJSON.Interval.PruningInterval) if err != nil { return errors.Wrap(err, "failed to parse pruning interval from JSON") } @@ -262,7 +237,7 @@ func (f *Fetcher) PruneOldImages() error { } // prune images older than the pruning interval - pruningDuration, err := parseDurationString(imageJSON.Interval.PruningIinterval) + pruningDuration, err := parseDurationString(imageJSON.Interval.PruningInterval) if err != nil { return errors.Wrap(err, "failed to parse pruning interval from JSON") } diff --git a/pkg/image/pull_policy_test.go b/pkg/image/pull_policy_test.go index 22cba1174b..246cf05eb6 100644 --- a/pkg/image/pull_policy_test.go +++ b/pkg/image/pull_policy_test.go @@ -34,16 +34,39 @@ func testPullPolicy(t *testing.T, when spec.G, it spec.S) { h.AssertEq(t, policy, image.PullIfNotPresent) }) - it("defaults to PullAlways, if empty string", func() { - policy, err := image.ParsePullPolicy("") + it("returns PullHourly for hourly", func() { + policy, err := image.ParsePullPolicy("hourly") h.AssertNil(t, err) - h.AssertEq(t, policy, image.PullAlways) + h.AssertEq(t, policy, image.PullHourly) + }) + + it("returns PullDaily for daily", func() { + policy, err := image.ParsePullPolicy("daily") + h.AssertNil(t, err) + h.AssertEq(t, policy, image.PullDaily) + }) + + it("returns PullWeekly for weekly", func() { + policy, err := image.ParsePullPolicy("weekly") + h.AssertNil(t, err) + h.AssertEq(t, policy, image.PullWeekly) + }) + + it("returns PullWithInterval for interval= format", func() { + policy, err := image.ParsePullPolicy("interval=4d") + h.AssertNil(t, err) + h.AssertEq(t, policy, image.PullWithInterval) }) it("returns error for unknown string", func() { _, err := image.ParsePullPolicy("fake-policy-here") h.AssertError(t, err, "invalid pull policy") }) + + it("returns error for invalid interval format", func() { + _, err := image.ParsePullPolicy("interval=invalid") + h.AssertError(t, err, "invalid interval format") + }) }) when("#String", func() { @@ -51,6 +74,10 @@ func testPullPolicy(t *testing.T, when spec.G, it spec.S) { h.AssertEq(t, image.PullAlways.String(), "always") h.AssertEq(t, image.PullNever.String(), "never") h.AssertEq(t, image.PullIfNotPresent.String(), "if-not-present") + h.AssertEq(t, image.PullHourly.String(), "hourly") + h.AssertEq(t, image.PullDaily.String(), "daily") + h.AssertEq(t, image.PullWeekly.String(), "weekly") + h.AssertContains(t, image.PullWithInterval.String(), "interval=") }) }) } From f6a698c09d3cda359b2ca26b961006abae5508a2 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 26 Feb 2024 22:18:04 +0000 Subject: [PATCH 06/11] build(deps): bump the go-dependencies group with 2 updates Bumps the go-dependencies group with 2 updates: [github.com/gdamore/tcell/v2](https://github.com/gdamore/tcell) and [golang.org/x/crypto](https://github.com/golang/crypto). Updates `github.com/gdamore/tcell/v2` from 2.7.0 to 2.7.1 - [Release notes](https://github.com/gdamore/tcell/releases) - [Changelog](https://github.com/gdamore/tcell/blob/main/CHANGESv2.md) - [Commits](https://github.com/gdamore/tcell/compare/v2.7.0...v2.7.1) Updates `golang.org/x/crypto` from 0.19.0 to 0.20.0 - [Commits](https://github.com/golang/crypto/compare/v0.19.0...v0.20.0) --- updated-dependencies: - dependency-name: github.com/gdamore/tcell/v2 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: go-dependencies - dependency-name: golang.org/x/crypto dependency-type: direct:production update-type: version-update:semver-minor dependency-group: go-dependencies ... Signed-off-by: dependabot[bot] --- go.mod | 11 ++++++++++- go.sum | 2 ++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index d074fe0eb6..318b0a136e 100644 --- a/go.mod +++ b/go.mod @@ -11,7 +11,11 @@ require ( github.com/docker/docker v25.0.5+incompatible github.com/docker/go-connections v0.5.0 github.com/dustin/go-humanize v1.0.1 +<<<<<<< HEAD github.com/gdamore/tcell/v2 v2.7.4 +======= + github.com/gdamore/tcell/v2 v2.7.1 +>>>>>>> 2ec7d744 (build(deps): bump the go-dependencies group with 2 updates) github.com/go-git/go-git/v5 v5.11.0 github.com/golang/mock v1.6.0 github.com/google/go-cmp v0.6.0 @@ -29,9 +33,14 @@ require ( github.com/sabhiram/go-gitignore v0.0.0-20210923224102-525f6e181f06 github.com/sclevine/spec v1.4.0 github.com/spf13/cobra v1.8.0 +<<<<<<< HEAD golang.org/x/crypto v0.21.0 golang.org/x/mod v0.16.0 - golang.org/x/oauth2 v0.18.0 +======= + golang.org/x/crypto v0.20.0 + golang.org/x/mod v0.15.0 +>>>>>>> 2ec7d744 (build(deps): bump the go-dependencies group with 2 updates) + golang.org/x/oauth2 v0.17.0 golang.org/x/sync v0.6.0 golang.org/x/sys v0.18.0 golang.org/x/term v0.18.0 diff --git a/go.sum b/go.sum index 88c9cf662a..32497cc93e 100644 --- a/go.sum +++ b/go.sum @@ -483,6 +483,7 @@ golang.org/x/sys v0.2.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.3.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.17.0 h1:25cE3gD+tdBA7lp7QfhuV+rJiE9YXTcS3VG1SqssI/Y= golang.org/x/sys v0.17.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.18.0 h1:DBdB3niSjOA/O0blCZBqDefyWNYveAYMNF1Wum0DYQ4= golang.org/x/sys v0.18.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= @@ -493,6 +494,7 @@ golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuX golang.org/x/term v0.2.0/go.mod h1:TVmDHMZPmdnySmBfhjOoOdhjzdE1h4u1VwSiw2l1Nuc= golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k= golang.org/x/term v0.6.0/go.mod h1:m6U89DPEgQRMq3DNkDClhWw02AUbt2daBVO4cn4Hv9U= +golang.org/x/term v0.17.0 h1:mkTF7LCd6WGJNL3K1Ad7kwxNfYAW6a8a8QqtMblp/4U= golang.org/x/term v0.17.0/go.mod h1:lLRBjIVuehSbZlaOtGMbcMncT+aqLLLmKrsjNrUguwk= golang.org/x/term v0.18.0 h1:FcHjZXDMxI8mM3nwhX9HlKop4C0YQvCVCdwYl2wOtE8= golang.org/x/term v0.18.0/go.mod h1:ILwASektA3OnRv7amZ1xhE/KTR+u50pbXfZ03+6Nx58= From 54864e7c8cd733d443298034dbbdd9001f6c0d4f Mon Sep 17 00:00:00 2001 From: Parthiba-Hazra Date: Tue, 5 Mar 2024 00:10:34 +0530 Subject: [PATCH 07/11] Test: Add test cases for `pack config prune-interval` Signed-off-by: Parthiba-Hazra --- go.mod | 21 +--- go.sum | 26 ++--- internal/commands/config_prune_interval.go | 7 +- .../commands/config_prune_interval_test.go | 106 ++++++++++++++++++ internal/commands/config_pull_policy_test.go | 12 -- pkg/image/fetcher.go | 6 +- pkg/image/pull_policy.go | 3 +- 7 files changed, 135 insertions(+), 46 deletions(-) create mode 100644 internal/commands/config_prune_interval_test.go diff --git a/go.mod b/go.mod index 318b0a136e..d89e014a75 100644 --- a/go.mod +++ b/go.mod @@ -11,11 +11,7 @@ require ( github.com/docker/docker v25.0.5+incompatible github.com/docker/go-connections v0.5.0 github.com/dustin/go-humanize v1.0.1 -<<<<<<< HEAD - github.com/gdamore/tcell/v2 v2.7.4 -======= github.com/gdamore/tcell/v2 v2.7.1 ->>>>>>> 2ec7d744 (build(deps): bump the go-dependencies group with 2 updates) github.com/go-git/go-git/v5 v5.11.0 github.com/golang/mock v1.6.0 github.com/google/go-cmp v0.6.0 @@ -33,13 +29,8 @@ require ( github.com/sabhiram/go-gitignore v0.0.0-20210923224102-525f6e181f06 github.com/sclevine/spec v1.4.0 github.com/spf13/cobra v1.8.0 -<<<<<<< HEAD - golang.org/x/crypto v0.21.0 - golang.org/x/mod v0.16.0 -======= golang.org/x/crypto v0.20.0 golang.org/x/mod v0.15.0 ->>>>>>> 2ec7d744 (build(deps): bump the go-dependencies group with 2 updates) golang.org/x/oauth2 v0.17.0 golang.org/x/sync v0.6.0 golang.org/x/sys v0.18.0 @@ -137,12 +128,12 @@ require ( github.com/spf13/pflag v1.0.5 // indirect github.com/vbatts/tar-split v0.11.5 // indirect github.com/xanzy/ssh-agent v0.3.3 // indirect - go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.48.0 // indirect - go.opentelemetry.io/otel v1.23.0 // indirect - go.opentelemetry.io/otel/metric v1.23.0 // indirect - go.opentelemetry.io/otel/trace v1.23.0 // indirect - golang.org/x/net v0.22.0 // indirect - golang.org/x/tools v0.17.0 // indirect + go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.47.0 // indirect + go.opentelemetry.io/otel v1.22.0 // indirect + go.opentelemetry.io/otel/metric v1.22.0 // indirect + go.opentelemetry.io/otel/trace v1.22.0 // indirect + golang.org/x/net v0.21.0 // indirect + golang.org/x/tools v0.18.0 // indirect google.golang.org/appengine v1.6.8 // indirect google.golang.org/protobuf v1.33.0 // indirect gopkg.in/warnings.v0 v0.1.2 // indirect diff --git a/go.sum b/go.sum index 32497cc93e..17ca08fc80 100644 --- a/go.sum +++ b/go.sum @@ -154,8 +154,8 @@ github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMo github.com/gdamore/encoding v1.0.0 h1:+7OoQ1Bc6eTm5niUzBa0Ctsh6JbMW6Ra+YNuAtDBdko= github.com/gdamore/encoding v1.0.0/go.mod h1:alR0ol34c49FCSBLjhosxzcPHQbf2trDkoo5dl+VrEg= github.com/gdamore/tcell/v2 v2.4.1-0.20210905002822-f057f0a857a1/go.mod h1:Az6Jt+M5idSED2YPGtwnfJV0kXohgdCBPmHGSYc1r04= -github.com/gdamore/tcell/v2 v2.7.4 h1:sg6/UnTM9jGpZU+oFYAsDahfchWAFW8Xx2yFinNSAYU= -github.com/gdamore/tcell/v2 v2.7.4/go.mod h1:dSXtXTSK0VsW1biw65DZLZ2NKr7j0qP/0J7ONmsraWg= +github.com/gdamore/tcell/v2 v2.7.1 h1:TiCcmpWHiAU7F0rA2I3S2Y4mmLmO9KHxJ7E1QhYzQbc= +github.com/gdamore/tcell/v2 v2.7.1/go.mod h1:dSXtXTSK0VsW1biw65DZLZ2NKr7j0qP/0J7ONmsraWg= github.com/gliderlabs/ssh v0.3.5 h1:OcaySEmAQJgyYcArR+gGGTHCyE7nvhEMTlYY+Dp8CpY= github.com/gliderlabs/ssh v0.3.5/go.mod h1:8XB4KraRrX39qHhT6yxPsHedjA08I/uBVwj4xC+/+z4= github.com/go-git/gcfg v1.5.1-0.20230307220236-3a3c6141e376 h1:+zs/tPmkDkHx3U66DAb0lQFJrpS6731Oaa12ikc+DiI= @@ -417,15 +417,15 @@ golang.org/x/crypto v0.0.0-20220722155217-630584e8d5aa/go.mod h1:IxCIyHEi3zRg3s0 golang.org/x/crypto v0.3.1-0.20221117191849-2c476679df9a/go.mod h1:hebNnKkNXi2UzZN1eVRvBB7co0a+JxK6XbPiWVs/3J4= golang.org/x/crypto v0.6.0/go.mod h1:OFC/31mSvZgRz0V1QTNCzfAI1aIRzbiufJtkMIlEp58= golang.org/x/crypto v0.7.0/go.mod h1:pYwdfH91IfpZVANVyUOhSIPZaFoJGxTFbZhFTx+dXZU= -golang.org/x/crypto v0.21.0 h1:X31++rzVUdKhX5sWmSOFZxx8UW/ldWx55cbf08iNAMA= -golang.org/x/crypto v0.21.0/go.mod h1:0BP7YvVV9gBbVKyeTG0Gyn+gZm94bibOW5BjDEYAOMs= +golang.org/x/crypto v0.20.0 h1:jmAMJJZXr5KiCw05dfYK9QnqaqKLYXijU23lsEdcQqg= +golang.org/x/crypto v0.20.0/go.mod h1:Xwo95rrVNIoSMx9wa1JroENMToLWn3RNVrTBpLHgZPQ= golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= -golang.org/x/mod v0.16.0 h1:QX4fJ0Rr5cPQCF7O9lh9Se4pmwfwskqZfq5moyldzic= -golang.org/x/mod v0.16.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c= +golang.org/x/mod v0.15.0 h1:SernR4v+D55NyBH2QiEQrlBAnj1ECL6AGrA5+dPaMY8= +golang.org/x/mod v0.15.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c= golang.org/x/net v0.0.0-20180906233101-161cd47e91fd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20181114220301-adae6a3d119a/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= @@ -441,11 +441,11 @@ golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug golang.org/x/net v0.2.0/go.mod h1:KqCZLdyyvdV855qA2rE3GC2aiw5xGR5TEjj8smXukLY= golang.org/x/net v0.6.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs= golang.org/x/net v0.8.0/go.mod h1:QVkue5JL9kW//ek3r6jTKnTFis1tRmNAW2P1shuFdJc= -golang.org/x/net v0.22.0 h1:9sGLhx7iRIHEiX0oAJ3MRZMUCElJgy7Br1nO+AMN3Tc= -golang.org/x/net v0.22.0/go.mod h1:JKghWKKOSdJwpW2GEx0Ja7fmaKnMsbu+MWVZTokSYmg= +golang.org/x/net v0.21.0 h1:AQyQV4dYCvJ7vGmJyKki9+PBdyvhkSd8EIx/qb0AYv4= +golang.org/x/net v0.21.0/go.mod h1:bIjVDfnllIU7BJ2DNgfnXvpSvtn8VRwhlsaeUTyUS44= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= -golang.org/x/oauth2 v0.18.0 h1:09qnuIAgzdx1XplqJvW6CQqMCtGZykZWcXzPMPUusvI= -golang.org/x/oauth2 v0.18.0/go.mod h1:Wf7knwG0MPoWIMMBgFlEaSUDaKskp0dCfrlJRJXbBi8= +golang.org/x/oauth2 v0.17.0 h1:6m3ZPmLEFdVxKKWnKq4VqZ60gutO35zm+zrAHVmHyDQ= +golang.org/x/oauth2 v0.17.0/go.mod h1:OzPDGQiuQMguemayvdylqddI7qcD9lnSDb+1FiwQ5HA= golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= @@ -483,7 +483,6 @@ golang.org/x/sys v0.2.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.3.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.17.0 h1:25cE3gD+tdBA7lp7QfhuV+rJiE9YXTcS3VG1SqssI/Y= golang.org/x/sys v0.17.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.18.0 h1:DBdB3niSjOA/O0blCZBqDefyWNYveAYMNF1Wum0DYQ4= golang.org/x/sys v0.18.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= @@ -494,7 +493,6 @@ golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuX golang.org/x/term v0.2.0/go.mod h1:TVmDHMZPmdnySmBfhjOoOdhjzdE1h4u1VwSiw2l1Nuc= golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k= golang.org/x/term v0.6.0/go.mod h1:m6U89DPEgQRMq3DNkDClhWw02AUbt2daBVO4cn4Hv9U= -golang.org/x/term v0.17.0 h1:mkTF7LCd6WGJNL3K1Ad7kwxNfYAW6a8a8QqtMblp/4U= golang.org/x/term v0.17.0/go.mod h1:lLRBjIVuehSbZlaOtGMbcMncT+aqLLLmKrsjNrUguwk= golang.org/x/term v0.18.0 h1:FcHjZXDMxI8mM3nwhX9HlKop4C0YQvCVCdwYl2wOtE8= golang.org/x/term v0.18.0/go.mod h1:ILwASektA3OnRv7amZ1xhE/KTR+u50pbXfZ03+6Nx58= @@ -518,8 +516,8 @@ golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4f golang.org/x/tools v0.1.1/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU= -golang.org/x/tools v0.17.0 h1:FvmRgNOcs3kOa+T20R1uhfP9F6HgG2mfxDv1vrx1Htc= -golang.org/x/tools v0.17.0/go.mod h1:xsh6VxdV005rRVaS6SSAf9oiAqljS7UZUacMZ8Bnsps= +golang.org/x/tools v0.18.0 h1:k8NLag8AGHnn+PHbl7g43CtqZAwG60vZkLqgyZgIHgQ= +golang.org/x/tools v0.18.0/go.mod h1:GL7B4CwcLLeo59yx/9UWWuNOW1n3VZ4f5axWfML7Lcg= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= diff --git a/internal/commands/config_prune_interval.go b/internal/commands/config_prune_interval.go index 9e1d8513ad..a4b00d9c0e 100644 --- a/internal/commands/config_prune_interval.go +++ b/internal/commands/config_prune_interval.go @@ -5,12 +5,13 @@ import ( "fmt" "regexp" + "github.com/pkg/errors" + "github.com/spf13/cobra" + "github.com/buildpacks/pack/internal/config" "github.com/buildpacks/pack/internal/style" "github.com/buildpacks/pack/pkg/image" "github.com/buildpacks/pack/pkg/logging" - "github.com/pkg/errors" - "github.com/spf13/cobra" ) func ConfigPruneInterval(logger logging.Logger, cfg config.Config, cfgPath string) *cobra.Command { @@ -30,7 +31,7 @@ func ConfigPruneInterval(logger logging.Logger, cfg config.Config, cfgPath strin switch { case unset: if len(args) > 0 { - return errors.Errorf("prune inteval and --unset cannot be specified simultaneously") + return errors.Errorf("prune interval and --unset cannot be specified simultaneously") } imageJSON, err := image.ReadImageJSON(logger) if err != nil { diff --git a/internal/commands/config_prune_interval_test.go b/internal/commands/config_prune_interval_test.go new file mode 100644 index 0000000000..4cedb231dc --- /dev/null +++ b/internal/commands/config_prune_interval_test.go @@ -0,0 +1,106 @@ +package commands_test + +import ( + "bytes" + "os" + "path/filepath" + "testing" + + "github.com/sclevine/spec" + "github.com/sclevine/spec/report" + "github.com/spf13/cobra" + + "github.com/buildpacks/pack/internal/commands" + "github.com/buildpacks/pack/internal/config" + "github.com/buildpacks/pack/pkg/logging" + h "github.com/buildpacks/pack/testhelpers" +) + +func TestConfigPruneInterval(t *testing.T) { + spec.Run(t, "ConfigPruneIntervalCommand", testConfigPruneIntervalCommand, spec.Random(), spec.Report(report.Terminal{})) +} + +func testConfigPruneIntervalCommand(t *testing.T, when spec.G, it spec.S) { + var ( + command *cobra.Command + logger logging.Logger + outBuf bytes.Buffer + tempPackHome string + configFile string + assert = h.NewAssertionManager(t) + cfg = config.Config{} + ) + + it.Before(func() { + logger = logging.NewLogWithWriters(&outBuf, &outBuf) + tempPackHome, _ = os.MkdirTemp("", "pack-home") + configFile = filepath.Join(tempPackHome, "config.toml") + + command = commands.ConfigPruneInterval(logger, cfg, configFile) + }) + + it.After(func() { + _ = os.RemoveAll(tempPackHome) + }) + + when("#ConfigPruneInterval", func() { + when("no arguments are provided", func() { + it("lists the current pruning interval", func() { + command.SetArgs([]string{}) + + err := command.Execute() + + assert.Nil(err) + assert.Contains(outBuf.String(), "The current prune interval is") + }) + }) + + when("an argument is provided", func() { + when("argument is valid", func() { + it("sets the provided interval as the pruning interval", func() { + interval := "5d" + command.SetArgs([]string{interval}) + + err := command.Execute() + + assert.Nil(err) + assert.Contains(outBuf.String(), "Successfully set") + }) + }) + + when("argument is invalid", func() { + it("returns an error", func() { + interval := "invalid" + command.SetArgs([]string{interval}) + + err := command.Execute() + + assert.Error(err) + assert.Contains(err.Error(), "invalid interval format") + }) + }) + }) + + when("--unset flag is provided", func() { + it("unsets the pruning interval", func() { + command.SetArgs([]string{"--unset"}) + + err := command.Execute() + + assert.Nil(err) + assert.Contains(outBuf.String(), "Successfully unset pruning interval") + }) + }) + + when("both interval and --unset flag are provided", func() { + it("returns an error", func() { + command.SetArgs([]string{"5d", "--unset"}) + + err := command.Execute() + + assert.Error(err) + assert.Contains(err.Error(), "prune interval and --unset cannot be specified simultaneously") + }) + }) + }) +} diff --git a/internal/commands/config_pull_policy_test.go b/internal/commands/config_pull_policy_test.go index 37b9ecfc01..b913bdf35b 100644 --- a/internal/commands/config_pull_policy_test.go +++ b/internal/commands/config_pull_policy_test.go @@ -200,12 +200,6 @@ func testConfigPullPolicyCommand(t *testing.T, when spec.G, it spec.S) { assert.Nil(err) assert.Equal(readCfg.PullPolicy, "never") }) - it("returns clear error if fails to write", func() { - assert.Nil(os.WriteFile(configFile, []byte("something"), 0001)) - command := commands.ConfigPullPolicy(logger, cfg, configFile) - command.SetArgs([]string{"if-not-present"}) - assert.ErrorContains(command.Execute(), "writing config to") - }) }) }) when("unset", func() { @@ -220,12 +214,6 @@ func testConfigPullPolicyCommand(t *testing.T, when spec.G, it spec.S) { assert.Nil(err) assert.Equal(cfg.PullPolicy, "") }) - it("returns clear error if fails to write", func() { - assert.Nil(os.WriteFile(configFile, []byte("something"), 0001)) - command := commands.ConfigPullPolicy(logger, config.Config{PullPolicy: "never"}, configFile) - command.SetArgs([]string{"--unset"}) - assert.ErrorContains(command.Execute(), "writing config to") - }) }) when("--unset and policy to set is provided", func() { it("errors", func() { diff --git a/pkg/image/fetcher.go b/pkg/image/fetcher.go index 247e88a4cd..363669baeb 100644 --- a/pkg/image/fetcher.go +++ b/pkg/image/fetcher.go @@ -41,6 +41,10 @@ type ImagePullChecker interface { ReadImageJSON(l logging.Logger) (*ImageJSON, error) } +func intervalPolicy(options FetchOptions) bool { + return options.PullPolicy == PullWithInterval || options.PullPolicy == PullHourly || options.PullPolicy == PullDaily || options.PullPolicy == PullWeekly +} + type PullChecker struct { logger logging.Logger } @@ -167,7 +171,7 @@ func (f *Fetcher) Fetch(ctx context.Context, name string, options FetchOptions) return nil, err } - if options.PullPolicy == PullWithInterval || options.PullPolicy == PullHourly || options.PullPolicy == PullDaily || options.PullPolicy == PullWeekly { + if intervalPolicy(options) { // Update image pull record in the JSON file if err := f.updateImagePullRecord(name, time.Now().Format(time.RFC3339)); err != nil { return nil, err diff --git a/pkg/image/pull_policy.go b/pkg/image/pull_policy.go index 98c667f2ae..1ba08303ff 100644 --- a/pkg/image/pull_policy.go +++ b/pkg/image/pull_policy.go @@ -11,8 +11,9 @@ import ( "strings" "time" - "github.com/buildpacks/pack/pkg/logging" "github.com/pkg/errors" + + "github.com/buildpacks/pack/pkg/logging" ) // PullPolicy defines a policy for how to manage images From 6ad2f0143927cdf30e861f54db91436ac17c4737 Mon Sep 17 00:00:00 2001 From: Parthiba-Hazra Date: Tue, 5 Mar 2024 13:56:28 +0530 Subject: [PATCH 08/11] Test: Update the fetcher test cases Signed-off-by: Parthiba-Hazra --- pkg/image/fetcher.go | 22 +++-- pkg/image/fetcher_test.go | 163 ++++++++++++++++++++++++++++++-------- pkg/image/pull_policy.go | 4 +- 3 files changed, 147 insertions(+), 42 deletions(-) diff --git a/pkg/image/fetcher.go b/pkg/image/fetcher.go index 363669baeb..6f584ec843 100644 --- a/pkg/image/fetcher.go +++ b/pkg/image/fetcher.go @@ -39,6 +39,8 @@ type LayoutOption struct { type ImagePullChecker interface { CheckImagePullInterval(imageID string, l logging.Logger) (bool, error) ReadImageJSON(l logging.Logger) (*ImageJSON, error) + PruneOldImages(l logging.Logger, f *Fetcher) error + UpdateImagePullRecord(l logging.Logger, imageID string, timestamp string) error } func intervalPolicy(options FetchOptions) bool { @@ -148,7 +150,7 @@ func (f *Fetcher) Fetch(ctx context.Context, name string, options FetchOptions) return img, err } - err = f.PruneOldImages() + err = f.imagePullChecker.PruneOldImages(f.logger, f) if err != nil { f.logger.Warnf("Failed to prune images, %s", err) } @@ -173,7 +175,7 @@ func (f *Fetcher) Fetch(ctx context.Context, name string, options FetchOptions) if intervalPolicy(options) { // Update image pull record in the JSON file - if err := f.updateImagePullRecord(name, time.Now().Format(time.RFC3339)); err != nil { + if err := f.imagePullChecker.UpdateImagePullRecord(f.logger, name, time.Now().Format(time.RFC3339)); err != nil { return nil, err } } @@ -305,8 +307,8 @@ func (w *colorizedWriter) Write(p []byte) (n int, err error) { return w.writer.Write([]byte(msg)) } -func (f *Fetcher) updateImagePullRecord(imageID, timestamp string) error { - imageJSON, err := ReadImageJSON(f.logger) +func UpdateImagePullRecord(l logging.Logger, imageID string, timestamp string) error { + imageJSON, err := ReadImageJSON(l) if err != nil { return err } @@ -330,11 +332,19 @@ func (f *Fetcher) updateImagePullRecord(imageID, timestamp string) error { } func (c *PullChecker) CheckImagePullInterval(imageID string, l logging.Logger) (bool, error) { - return CheckImagePullInterval(imageID, c.logger) + return CheckImagePullInterval(imageID, l) } func (c *PullChecker) ReadImageJSON(l logging.Logger) (*ImageJSON, error) { - return ReadImageJSON(c.logger) + return ReadImageJSON(l) +} + +func (c *PullChecker) PruneOldImages(l logging.Logger, f *Fetcher) error { + return PruneOldImages(l, f) +} + +func (c *PullChecker) UpdateImagePullRecord(l logging.Logger, imageID string, timestamp string) error { + return UpdateImagePullRecord(l, imageID, timestamp) } func CheckImagePullInterval(imageID string, l logging.Logger) (bool, error) { diff --git a/pkg/image/fetcher_test.go b/pkg/image/fetcher_test.go index d1ea37e35c..f96cacd2c6 100644 --- a/pkg/image/fetcher_test.go +++ b/pkg/image/fetcher_test.go @@ -8,6 +8,7 @@ import ( "path/filepath" "runtime" "testing" + "time" "github.com/buildpacks/imgutil" @@ -34,6 +35,8 @@ type MockPullChecker struct { *image.PullChecker MockCheckImagePullInterval func(imageID string, l logging.Logger) (bool, error) MockReadImageJSON func(l logging.Logger) (*image.ImageJSON, error) + MockPruneOldImages func(l logging.Logger, f *image.Fetcher) error + MockUpdateImagePullRecord func(l logging.Logger, imageID string, timestamp string) error } func NewMockImagePullChecker(logger logging.Logger) *MockPullChecker { @@ -53,13 +56,38 @@ func (m *MockPullChecker) ReadImageJSON(l logging.Logger) (*image.ImageJSON, err if m.MockReadImageJSON != nil { return m.MockReadImageJSON(l) } - return &image.ImageJSON{ + + imageJSON = &image.ImageJSON{ + Interval: &image.Interval{ + PullingInterval: "7d", + PruningInterval: "7d", + LastPrune: "2023-01-01T00:00:00Z", + }, Image: &image.ImageData{ ImageIDtoTIME: map[string]string{ "repoName": "2023-01-01T00:00:00Z", }, }, - }, nil + } + + return imageJSON, nil +} + +func (m *MockPullChecker) PruneOldImages(l logging.Logger, f *image.Fetcher) error { + if m.MockPruneOldImages != nil { + return m.MockPruneOldImages(l, f) + } + + return nil +} + +func (m *MockPullChecker) UpdateImagePullRecord(l logging.Logger, imageID string, timestamp string) error { + if m.MockUpdateImagePullRecord != nil { + fmt.Printf("checking wheather its calling or not") + return m.MockUpdateImagePullRecord(l, imageID, timestamp) + } + + return nil } func TestFetcher(t *testing.T) { @@ -437,15 +465,24 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, l logging.Logger) (bool, error) { return false, nil } + + imageJSON = &image.ImageJSON{ + Interval: &image.Interval{ + PullingInterval: "7d", + PruningInterval: "7d", + LastPrune: "2023-01-01T00:00:00Z", + }, + Image: &image.ImageData{ + ImageIDtoTIME: map[string]string{ + repoName: "2023-01-01T00:00:00Z", + }, + }, + } + imageFetcher = image.NewFetcher(logging.NewLogWithWriters(&outBuf, &outBuf), docker, mockImagePullChecker) + mockImagePullChecker.MockReadImageJSON = func(l logging.Logger) (*image.ImageJSON, error) { - return &image.ImageJSON{ - Image: &image.ImageData{ - ImageIDtoTIME: map[string]string{ - repoName: "2023-01-01T00:00:00Z", - }, - }, - }, nil + return imageJSON, nil } }) @@ -459,7 +496,8 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { h.AssertError(t, err, fmt.Sprintf("image '%s' does not exist on the daemon", repoName)) imageJSON, err = mockImagePullChecker.ReadImageJSON(logger) h.AssertNil(t, err) - h.AssertNil(t, imageJSON.Image.ImageIDtoTIME) + _, exists := imageJSON.Image.ImageIDtoTIME[repoName] + h.AssertEq(t, exists, false) }) }) @@ -472,30 +510,62 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, l logging.Logger) (bool, error) { return true, nil } + + imageJSON = &image.ImageJSON{ + Interval: &image.Interval{ + PullingInterval: "7d", + PruningInterval: "7d", + LastPrune: "2023-01-01T00:00:00Z", + }, + Image: &image.ImageData{ + ImageIDtoTIME: map[string]string{ + repoName: "2023-01-01T00:00:00Z", + }, + }, + } + + mockImagePullChecker.MockReadImageJSON = func(l logging.Logger) (*image.ImageJSON, error) { + return imageJSON, nil + } + + mockImagePullChecker.MockUpdateImagePullRecord = func(l logging.Logger, imageID string, timestamp string) error { + imageJSON, _ = mockImagePullChecker.ReadImageJSON(l) + imageJSON.Image.ImageIDtoTIME[repoName] = timestamp + return nil + } + imageFetcher = image.NewFetcher(logging.NewLogWithWriters(&outBuf, &outBuf), docker, mockImagePullChecker) }) it.After(func() { mockImagePullChecker.MockCheckImagePullInterval = nil + mockImagePullChecker.MockReadImageJSON = nil + mockImagePullChecker.MockUpdateImagePullRecord = nil h.DockerRmi(docker, repoName) }) it("pulls the remote image and returns it", func() { - fetchedImg, err := imageFetcher.Fetch(context.TODO(), repoName, image.FetchOptions{Daemon: true, PullPolicy: image.PullIfNotPresent}) + beforeFetch, _ := time.Parse(time.RFC3339, imageJSON.Image.ImageIDtoTIME[repoName]) + fmt.Printf("before fetch: %v\n", imageJSON.Image.ImageIDtoTIME[repoName]) + _, err := imageFetcher.Fetch(context.TODO(), repoName, image.FetchOptions{Daemon: true, PullPolicy: image.PullWeekly}) h.AssertNil(t, err) - fetchedImgLabel, err := fetchedImg.Label(label) - h.AssertNil(t, err) - h.AssertEq(t, fetchedImgLabel, remoteImgLabel) + imageJSON, _ = mockImagePullChecker.ReadImageJSON(logger) + + afterFetch, _ := time.Parse(time.RFC3339, imageJSON.Image.ImageIDtoTIME[repoName]) + fmt.Printf("after fetch: %v\n", imageJSON.Image.ImageIDtoTIME[repoName]) + diff := beforeFetch.Before(afterFetch) + h.AssertEq(t, diff, true) }) }) when("there is a local image and CheckImagePullInterval returns false", func() { it.Before(func() { - img, err := local.NewImage(repoName, docker) + localImg, err := local.NewImage(repoName, docker) h.AssertNil(t, err) + h.AssertNil(t, localImg.SetLabel(label, "2")) - h.AssertNil(t, img.Save()) + h.AssertNil(t, localImg.Save()) mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, l logging.Logger) (bool, error) { return true, nil } @@ -508,13 +578,19 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { }) it("returns the local image", func() { - _, err := imageFetcher.Fetch(context.TODO(), repoName, image.FetchOptions{Daemon: true, PullPolicy: image.PullIfNotPresent}) + fetchedImg, err := imageFetcher.Fetch(context.TODO(), repoName, image.FetchOptions{Daemon: true, PullPolicy: image.PullIfNotPresent}) + h.AssertNil(t, err) + + fetchedImgLabel, err := fetchedImg.Label(label) h.AssertNil(t, err) + h.AssertEq(t, fetchedImgLabel, "2") }) }) }) - when("there is no a remote image", func() { + when("there is no remote image", func() { + var label string + when("there is no local image and CheckImagePullInterval returns true", func() { it.Before(func() { mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, l logging.Logger) (bool, error) { @@ -538,15 +614,23 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, l logging.Logger) (bool, error) { return false, nil } + + imageJSON = &image.ImageJSON{ + Interval: &image.Interval{ + PullingInterval: "7d", + PruningInterval: "7d", + LastPrune: "2023-01-01T00:00:00Z", + }, + Image: &image.ImageData{ + ImageIDtoTIME: map[string]string{ + repoName: "2023-01-01T00:00:00Z", + }, + }, + } + imageFetcher = image.NewFetcher(logging.NewLogWithWriters(&outBuf, &outBuf), docker, mockImagePullChecker) mockImagePullChecker.MockReadImageJSON = func(l logging.Logger) (*image.ImageJSON, error) { - return &image.ImageJSON{ - Image: &image.ImageData{ - ImageIDtoTIME: map[string]string{ - repoName: "2023-01-01T00:00:00Z", - }, - }, - }, nil + return imageJSON, nil } }) @@ -560,16 +644,18 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { h.AssertError(t, err, fmt.Sprintf("image '%s' does not exist on the daemon", repoName)) imageJSON, err = mockImagePullChecker.ReadImageJSON(logger) h.AssertNil(t, err) - h.AssertNil(t, imageJSON.Image.ImageIDtoTIME) + _, exists := imageJSON.Image.ImageIDtoTIME[repoName] + h.AssertEq(t, exists, false) }) }) when("there is a local image and CheckImagePullInterval returns true", func() { it.Before(func() { - img, err := local.NewImage(repoName, docker) + localImg, err := local.NewImage(repoName, docker) h.AssertNil(t, err) + h.AssertNil(t, localImg.SetLabel(label, "2")) - h.AssertNil(t, img.Save()) + h.AssertNil(t, localImg.Save()) mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, l logging.Logger) (bool, error) { return true, nil } @@ -581,18 +667,23 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { h.DockerRmi(docker, repoName) }) - it("try to pull the remote image and returns error", func() { - _, err := imageFetcher.Fetch(context.TODO(), repoName, image.FetchOptions{Daemon: true, PullPolicy: image.PullIfNotPresent}) - h.AssertNotNil(t, err) + it("try to pull the remote image and returns local image", func() { + fetchedImg, err := imageFetcher.Fetch(context.TODO(), repoName, image.FetchOptions{Daemon: true, PullPolicy: image.PullIfNotPresent}) + h.AssertNil(t, err) + + fetchedImgLabel, err := fetchedImg.Label(label) + h.AssertNil(t, err) + h.AssertEq(t, fetchedImgLabel, "2") }) }) when("there is a local image and CheckImagePullInterval returns false", func() { it.Before(func() { - img, err := local.NewImage(repoName, docker) + localImg, err := local.NewImage(repoName, docker) h.AssertNil(t, err) + h.AssertNil(t, localImg.SetLabel(label, "2")) - h.AssertNil(t, img.Save()) + h.AssertNil(t, localImg.Save()) mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, l logging.Logger) (bool, error) { return true, nil } @@ -605,8 +696,12 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { }) it("returns the local image", func() { - _, err := imageFetcher.Fetch(context.TODO(), repoName, image.FetchOptions{Daemon: true, PullPolicy: image.PullIfNotPresent}) + fetchedImg, err := imageFetcher.Fetch(context.TODO(), repoName, image.FetchOptions{Daemon: true, PullPolicy: image.PullIfNotPresent}) + h.AssertNil(t, err) + + fetchedImgLabel, err := fetchedImg.Label(label) h.AssertNil(t, err) + h.AssertEq(t, fetchedImgLabel, "2") }) }) }) diff --git a/pkg/image/pull_policy.go b/pkg/image/pull_policy.go index 1ba08303ff..0322ee0e7e 100644 --- a/pkg/image/pull_policy.go +++ b/pkg/image/pull_policy.go @@ -214,8 +214,8 @@ func parseDurationString(durationStr string) (time.Duration, error) { return time.Duration(totalMinutes) * time.Minute, nil } -func (f *Fetcher) PruneOldImages() error { - imageJSON, err := ReadImageJSON(f.logger) +func PruneOldImages(l logging.Logger, f *Fetcher) error { + imageJSON, err := ReadImageJSON(l) if err != nil { return err } From 1b2311f08ccb0deb1059f6f4e8ec8774bbb3236d Mon Sep 17 00:00:00 2001 From: Parthiba-Hazra Date: Sun, 10 Mar 2024 23:02:05 +0530 Subject: [PATCH 09/11] refactor: Refactor image JSON handling into a dedicated manager - This commit introduces an ImagePullPolicyManager struct responsible for handling image JSON operations, including reading and writing the image.json file. The ReadImageJSON and WriteFile functions have been refactored to methods of this manager. Signed-off-by: Parthiba-Hazra --- internal/commands/build.go | 2 +- internal/commands/builder_create.go | 2 +- internal/commands/buildpack_package.go | 2 +- internal/commands/config_prune_interval.go | 26 ++-- internal/commands/config_pull_policy.go | 6 +- internal/commands/create_builder.go | 2 +- internal/commands/extension_package.go | 2 +- internal/commands/package_buildpack.go | 2 +- internal/commands/rebase.go | 2 +- pkg/client/client.go | 2 +- pkg/image/fetcher.go | 67 ++++------ pkg/image/fetcher_test.go | 78 ++++++----- pkg/image/pull_policy.go | 142 +++++++++++---------- pkg/image/pull_policy_test.go | 18 +-- 14 files changed, 172 insertions(+), 181 deletions(-) diff --git a/internal/commands/build.go b/internal/commands/build.go index d32f96cf8b..d04c74ab54 100644 --- a/internal/commands/build.go +++ b/internal/commands/build.go @@ -128,7 +128,7 @@ func Build(logger logging.Logger, cfg config.Config, packClient PackClient) *cob if stringPolicy == "" { stringPolicy = cfg.PullPolicy } - pullPolicy, err := image.ParsePullPolicy(stringPolicy) + pullPolicy, err := image.ParsePullPolicy(stringPolicy, logger) if err != nil { return errors.Wrapf(err, "parsing pull policy %s", flags.Policy) } diff --git a/internal/commands/builder_create.go b/internal/commands/builder_create.go index 56ac071fe2..d6604e3576 100644 --- a/internal/commands/builder_create.go +++ b/internal/commands/builder_create.go @@ -50,7 +50,7 @@ Creating a custom builder allows you to control what buildpacks are used and wha if stringPolicy == "" { stringPolicy = cfg.PullPolicy } - pullPolicy, err := image.ParsePullPolicy(stringPolicy) + pullPolicy, err := image.ParsePullPolicy(stringPolicy, logger) if err != nil { return errors.Wrapf(err, "parsing pull policy %s", flags.Policy) } diff --git a/internal/commands/buildpack_package.go b/internal/commands/buildpack_package.go index adb95c3e03..35f99ffc98 100644 --- a/internal/commands/buildpack_package.go +++ b/internal/commands/buildpack_package.go @@ -62,7 +62,7 @@ func BuildpackPackage(logger logging.Logger, cfg config.Config, packager Buildpa if stringPolicy == "" { stringPolicy = cfg.PullPolicy } - pullPolicy, err := image.ParsePullPolicy(stringPolicy) + pullPolicy, err := image.ParsePullPolicy(stringPolicy, logger) if err != nil { return errors.Wrap(err, "parsing pull policy") } diff --git a/internal/commands/config_prune_interval.go b/internal/commands/config_prune_interval.go index a4b00d9c0e..eae27bb307 100644 --- a/internal/commands/config_prune_interval.go +++ b/internal/commands/config_prune_interval.go @@ -1,7 +1,6 @@ package commands import ( - "encoding/json" "fmt" "regexp" @@ -17,6 +16,7 @@ import ( func ConfigPruneInterval(logger logging.Logger, cfg config.Config, cfgPath string) *cobra.Command { var unset bool var intervalRegex = regexp.MustCompile(`^(\d+d)?(\d+h)?(\d+m)?$`) + pullPolicyManager := image.NewPullPolicyManager(logger) cmd := &cobra.Command{ Use: "prune-interval", @@ -28,30 +28,31 @@ func ConfigPruneInterval(logger logging.Logger, cfg config.Config, cfgPath strin "* To unset the pruning interval, run `pack config prune-interval --unset`.\n" + fmt.Sprintf("Unsetting the pruning interval will reset the interval to the default, which is %s.", style.Symbol("7 days")), RunE: logError(logger, func(cmd *cobra.Command, args []string) error { + imageJSONPath, err := image.DefaultImageJSONPath() + if err != nil { + return err + } + switch { case unset: if len(args) > 0 { return errors.Errorf("prune interval and --unset cannot be specified simultaneously") } - imageJSON, err := image.ReadImageJSON(logger) + imageJSON, err := pullPolicyManager.Read(imageJSONPath) if err != nil { return err } oldPruneInterval := imageJSON.Interval.PruningInterval imageJSON.Interval.PruningInterval = "7d" - updatedJSON, err := json.MarshalIndent(imageJSON, "", " ") - if err != nil { - return errors.Wrap(err, "failed to marshal updated records") - } - err = image.WriteFile(updatedJSON) + err = pullPolicyManager.Write(imageJSON, imageJSONPath) if err != nil { return err } logger.Infof("Successfully unset pruning interval %s", style.Symbol(oldPruneInterval)) logger.Infof("Pruning interval has been set to %s", style.Symbol(imageJSON.Interval.PruningInterval)) case len(args) == 0: // list - imageJSON, err := image.ReadImageJSON(logger) + imageJSON, err := pullPolicyManager.Read(imageJSONPath) if err != nil { return err } @@ -64,7 +65,7 @@ func ConfigPruneInterval(logger logging.Logger, cfg config.Config, cfgPath strin default: // set newPruneInterval := args[0] - imageJSON, err := image.ReadImageJSON(logger) + imageJSON, err := pullPolicyManager.Read(imageJSONPath) if err != nil { return err } @@ -84,11 +85,8 @@ func ConfigPruneInterval(logger logging.Logger, cfg config.Config, cfgPath strin } imageJSON.Interval.PruningInterval = newPruneInterval - updatedJSON, err := json.MarshalIndent(imageJSON, "", " ") - if err != nil { - return errors.Wrap(err, "failed to marshal updated records") - } - err = image.WriteFile(updatedJSON) + + err = pullPolicyManager.Write(imageJSON, imageJSONPath) if err != nil { return err } diff --git a/internal/commands/config_pull_policy.go b/internal/commands/config_pull_policy.go index 54711b26d6..a5d1968b99 100644 --- a/internal/commands/config_pull_policy.go +++ b/internal/commands/config_pull_policy.go @@ -36,7 +36,7 @@ func ConfigPullPolicy(logger logging.Logger, cfg config.Config, cfgPath string) return errors.Wrapf(err, "writing config to %s", cfgPath) } - pullPolicy, err := image.ParsePullPolicy(cfg.PullPolicy) + pullPolicy, err := image.ParsePullPolicy(cfg.PullPolicy, logger) if err != nil { return err } @@ -44,7 +44,7 @@ func ConfigPullPolicy(logger logging.Logger, cfg config.Config, cfgPath string) logger.Infof("Successfully unset pull policy %s", style.Symbol(oldPullPolicy)) logger.Infof("Pull policy has been set to %s", style.Symbol(pullPolicy.String())) case len(args) == 0: // list - pullPolicy, err := image.ParsePullPolicy(cfg.PullPolicy) + pullPolicy, err := image.ParsePullPolicy(cfg.PullPolicy, logger) if err != nil { return err } @@ -58,7 +58,7 @@ func ConfigPullPolicy(logger logging.Logger, cfg config.Config, cfgPath string) return nil } - pullPolicy, err := image.ParsePullPolicy(newPullPolicy) + pullPolicy, err := image.ParsePullPolicy(newPullPolicy, logger) if err != nil { return err } diff --git a/internal/commands/create_builder.go b/internal/commands/create_builder.go index 9999392ef6..4ac3f6a09a 100644 --- a/internal/commands/create_builder.go +++ b/internal/commands/create_builder.go @@ -43,7 +43,7 @@ Creating a custom builder allows you to control what buildpacks are used and wha if stringPolicy == "" { stringPolicy = cfg.PullPolicy } - pullPolicy, err := image.ParsePullPolicy(stringPolicy) + pullPolicy, err := image.ParsePullPolicy(stringPolicy, logger) if err != nil { return errors.Wrapf(err, "parsing pull policy %s", flags.Policy) } diff --git a/internal/commands/extension_package.go b/internal/commands/extension_package.go index 0415823e57..cea78a629a 100644 --- a/internal/commands/extension_package.go +++ b/internal/commands/extension_package.go @@ -45,7 +45,7 @@ func ExtensionPackage(logger logging.Logger, cfg config.Config, packager Extensi stringPolicy = cfg.PullPolicy } - pullPolicy, err := image.ParsePullPolicy(stringPolicy) + pullPolicy, err := image.ParsePullPolicy(stringPolicy, logger) if err != nil { return errors.Wrap(err, "parsing pull policy") } diff --git a/internal/commands/package_buildpack.go b/internal/commands/package_buildpack.go index 1bd17c4561..a5fc9b6da9 100644 --- a/internal/commands/package_buildpack.go +++ b/internal/commands/package_buildpack.go @@ -41,7 +41,7 @@ func PackageBuildpack(logger logging.Logger, cfg config.Config, packager Buildpa if stringPolicy == "" { stringPolicy = cfg.PullPolicy } - pullPolicy, err := image.ParsePullPolicy(stringPolicy) + pullPolicy, err := image.ParsePullPolicy(stringPolicy, logger) if err != nil { return errors.Wrap(err, "parsing pull policy") } diff --git a/internal/commands/rebase.go b/internal/commands/rebase.go index d0622f9db7..f522163f54 100644 --- a/internal/commands/rebase.go +++ b/internal/commands/rebase.go @@ -33,7 +33,7 @@ func Rebase(logger logging.Logger, cfg config.Config, pack PackClient) *cobra.Co if stringPolicy == "" { stringPolicy = cfg.PullPolicy } - opts.PullPolicy, err = image.ParsePullPolicy(stringPolicy) + opts.PullPolicy, err = image.ParsePullPolicy(stringPolicy, logger) if err != nil { return errors.Wrapf(err, "parsing pull policy %s", stringPolicy) } diff --git a/pkg/client/client.go b/pkg/client/client.go index 9786b61e0b..ad974cd44b 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -231,7 +231,7 @@ func NewClient(opts ...Option) (*Client, error) { } if client.imageFetcher == nil { - imagePullChecker := image.NewPullChecker(client.logger) + imagePullChecker := image.NewPullPolicyManager(client.logger) client.imageFetcher = image.NewFetcher(client.logger, client.docker, imagePullChecker, image.WithRegistryMirrors(client.registryMirrors), image.WithKeychain(client.keychain)) } diff --git a/pkg/image/fetcher.go b/pkg/image/fetcher.go index 6f584ec843..9f26c2921e 100644 --- a/pkg/image/fetcher.go +++ b/pkg/image/fetcher.go @@ -37,22 +37,19 @@ type LayoutOption struct { } type ImagePullChecker interface { - CheckImagePullInterval(imageID string, l logging.Logger) (bool, error) - ReadImageJSON(l logging.Logger) (*ImageJSON, error) - PruneOldImages(l logging.Logger, f *Fetcher) error - UpdateImagePullRecord(l logging.Logger, imageID string, timestamp string) error + CheckImagePullInterval(imageID string, path string) (bool, error) + Read(path string) (*ImageJSON, error) + PruneOldImages(f *Fetcher) error + UpdateImagePullRecord(path string, imageID string, timestamp string) error + Write(imageJSON *ImageJSON, path string) error } func intervalPolicy(options FetchOptions) bool { return options.PullPolicy == PullWithInterval || options.PullPolicy == PullHourly || options.PullPolicy == PullDaily || options.PullPolicy == PullWeekly } -type PullChecker struct { - logger logging.Logger -} - -func NewPullChecker(logger logging.Logger) *PullChecker { - return &PullChecker{logger: logger} +func NewPullPolicyManager(logger logging.Logger) *ImagePullPolicyManager { + return &ImagePullPolicyManager{Logger: logger} } // WithRegistryMirrors supply your own mirrors for registry. @@ -119,6 +116,11 @@ func (f *Fetcher) Fetch(ctx context.Context, name string, options FetchOptions) return f.fetchRemoteImage(name) } + imageJSONpath, err := DefaultImageJSONPath() + if err != nil { + return nil, err + } + switch options.PullPolicy { case PullNever: img, err := f.fetchDaemonImage(name) @@ -129,28 +131,24 @@ func (f *Fetcher) Fetch(ctx context.Context, name string, options FetchOptions) return img, err } case PullWithInterval, PullDaily, PullHourly, PullWeekly: - pull, err := f.imagePullChecker.CheckImagePullInterval(name, f.logger) + pull, err := f.imagePullChecker.CheckImagePullInterval(name, imageJSONpath) if err != nil { f.logger.Warnf("failed to check pulling interval for image %s, %s", name, err) } if !pull { img, err := f.fetchDaemonImage(name) if errors.Is(err, ErrNotFound) { - imageJSON, _ := f.imagePullChecker.ReadImageJSON(f.logger) + imageJSON, _ := f.imagePullChecker.Read(imageJSONpath) delete(imageJSON.Image.ImageIDtoTIME, name) - updatedJSON, err := json.MarshalIndent(imageJSON, "", " ") - if err != nil { - f.logger.Errorf("failed to marshal updated records %s", err) - } - if err := WriteFile(updatedJSON); err != nil { + if err := f.imagePullChecker.Write(imageJSON, imageJSONpath); err != nil { f.logger.Errorf("failed to write updated image.json %s", err) } } return img, err } - err = f.imagePullChecker.PruneOldImages(f.logger, f) + err = f.imagePullChecker.PruneOldImages(f) if err != nil { f.logger.Warnf("Failed to prune images, %s", err) } @@ -175,7 +173,7 @@ func (f *Fetcher) Fetch(ctx context.Context, name string, options FetchOptions) if intervalPolicy(options) { // Update image pull record in the JSON file - if err := f.imagePullChecker.UpdateImagePullRecord(f.logger, name, time.Now().Format(time.RFC3339)); err != nil { + if err := f.imagePullChecker.UpdateImagePullRecord(imageJSONpath, name, time.Now().Format(time.RFC3339)); err != nil { return nil, err } } @@ -307,8 +305,8 @@ func (w *colorizedWriter) Write(p []byte) (n int, err error) { return w.writer.Write([]byte(msg)) } -func UpdateImagePullRecord(l logging.Logger, imageID string, timestamp string) error { - imageJSON, err := ReadImageJSON(l) +func (i *ImagePullPolicyManager) UpdateImagePullRecord(path string, imageID string, timestamp string) error { + imageJSON, err := i.Read(path) if err != nil { return err } @@ -318,12 +316,7 @@ func UpdateImagePullRecord(l logging.Logger, imageID string, timestamp string) e } imageJSON.Image.ImageIDtoTIME[imageID] = timestamp - updatedJSON, err := json.MarshalIndent(imageJSON, "", " ") - if err != nil { - return errors.New("failed to marshal updated records: " + err.Error()) - } - - err = WriteFile(updatedJSON) + err = i.Write(imageJSON, path) if err != nil { return err } @@ -331,24 +324,8 @@ func UpdateImagePullRecord(l logging.Logger, imageID string, timestamp string) e return nil } -func (c *PullChecker) CheckImagePullInterval(imageID string, l logging.Logger) (bool, error) { - return CheckImagePullInterval(imageID, l) -} - -func (c *PullChecker) ReadImageJSON(l logging.Logger) (*ImageJSON, error) { - return ReadImageJSON(l) -} - -func (c *PullChecker) PruneOldImages(l logging.Logger, f *Fetcher) error { - return PruneOldImages(l, f) -} - -func (c *PullChecker) UpdateImagePullRecord(l logging.Logger, imageID string, timestamp string) error { - return UpdateImagePullRecord(l, imageID, timestamp) -} - -func CheckImagePullInterval(imageID string, l logging.Logger) (bool, error) { - imageJSON, err := ReadImageJSON(l) +func (i *ImagePullPolicyManager) CheckImagePullInterval(imageID string, path string) (bool, error) { + imageJSON, err := i.Read(path) if err != nil { return false, err } diff --git a/pkg/image/fetcher_test.go b/pkg/image/fetcher_test.go index f96cacd2c6..f20ba35ace 100644 --- a/pkg/image/fetcher_test.go +++ b/pkg/image/fetcher_test.go @@ -32,29 +32,37 @@ var imageJSON *image.ImageJSON var mockImagePullChecker = NewMockImagePullChecker(logger) type MockPullChecker struct { - *image.PullChecker - MockCheckImagePullInterval func(imageID string, l logging.Logger) (bool, error) - MockReadImageJSON func(l logging.Logger) (*image.ImageJSON, error) - MockPruneOldImages func(l logging.Logger, f *image.Fetcher) error - MockUpdateImagePullRecord func(l logging.Logger, imageID string, timestamp string) error + *image.ImagePullPolicyManager + MockCheckImagePullInterval func(imageID string, path string) (bool, error) + MockRead func(path string) (*image.ImageJSON, error) + MockPruneOldImages func(f *image.Fetcher) error + MockUpdateImagePullRecord func(path string, imageID string, timestamp string) error + MockWrite func(imageJSON *image.ImageJSON, path string) error } func NewMockImagePullChecker(logger logging.Logger) *MockPullChecker { return &MockPullChecker{ - PullChecker: image.NewPullChecker(logger), + ImagePullPolicyManager: image.NewPullPolicyManager(logger), } } -func (m *MockPullChecker) CheckImagePullInterval(imageID string, l logging.Logger) (bool, error) { +func (m *MockPullChecker) CheckImagePullInterval(imageID string, path string) (bool, error) { if m.MockCheckImagePullInterval != nil { - return m.MockCheckImagePullInterval(imageID, l) + return m.MockCheckImagePullInterval(imageID, path) } return false, nil } -func (m *MockPullChecker) ReadImageJSON(l logging.Logger) (*image.ImageJSON, error) { - if m.MockReadImageJSON != nil { - return m.MockReadImageJSON(l) +func (m *MockPullChecker) Write(imageJSON *image.ImageJSON, path string) error { + if m.MockWrite != nil { + return m.MockWrite(imageJSON, path) + } + return nil +} + +func (m *MockPullChecker) Read(path string) (*image.ImageJSON, error) { + if m.MockRead != nil { + return m.MockRead(path) } imageJSON = &image.ImageJSON{ @@ -73,18 +81,18 @@ func (m *MockPullChecker) ReadImageJSON(l logging.Logger) (*image.ImageJSON, err return imageJSON, nil } -func (m *MockPullChecker) PruneOldImages(l logging.Logger, f *image.Fetcher) error { +func (m *MockPullChecker) PruneOldImages(f *image.Fetcher) error { if m.MockPruneOldImages != nil { - return m.MockPruneOldImages(l, f) + return m.MockPruneOldImages(f) } return nil } -func (m *MockPullChecker) UpdateImagePullRecord(l logging.Logger, imageID string, timestamp string) error { +func (m *MockPullChecker) UpdateImagePullRecord(path string, imageID string, timestamp string) error { if m.MockUpdateImagePullRecord != nil { fmt.Printf("checking wheather its calling or not") - return m.MockUpdateImagePullRecord(l, imageID, timestamp) + return m.MockUpdateImagePullRecord(path, imageID, timestamp) } return nil @@ -232,7 +240,7 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { var outCons *color.Console outCons, output = h.MockWriterAndOutput() logger = logging.NewLogWithWriters(outCons, outCons) - mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, l logging.Logger) (bool, error) { + mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, path string) (bool, error) { return true, nil } imageFetcher = image.NewFetcher(logger, docker, mockImagePullChecker) @@ -440,7 +448,7 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { when("there is no local image and CheckImagePullInterval returns true", func() { it.Before(func() { - mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, l logging.Logger) (bool, error) { + mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, path string) (bool, error) { return true, nil } imageFetcher = image.NewFetcher(logging.NewLogWithWriters(&outBuf, &outBuf), docker, mockImagePullChecker) @@ -462,7 +470,7 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { when("there is no local image and CheckImagePullInterval returns false", func() { it.Before(func() { - mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, l logging.Logger) (bool, error) { + mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, path string) (bool, error) { return false, nil } @@ -481,20 +489,20 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { imageFetcher = image.NewFetcher(logging.NewLogWithWriters(&outBuf, &outBuf), docker, mockImagePullChecker) - mockImagePullChecker.MockReadImageJSON = func(l logging.Logger) (*image.ImageJSON, error) { + mockImagePullChecker.MockRead = func(path string) (*image.ImageJSON, error) { return imageJSON, nil } }) it.After(func() { mockImagePullChecker.MockCheckImagePullInterval = nil - mockImagePullChecker.MockReadImageJSON = nil + mockImagePullChecker.MockRead = nil }) it("returns an error and deletes the image record", func() { _, err := imageFetcher.Fetch(context.TODO(), repoName, image.FetchOptions{Daemon: true, PullPolicy: image.PullWeekly}) h.AssertError(t, err, fmt.Sprintf("image '%s' does not exist on the daemon", repoName)) - imageJSON, err = mockImagePullChecker.ReadImageJSON(logger) + imageJSON, err = mockImagePullChecker.Read("") h.AssertNil(t, err) _, exists := imageJSON.Image.ImageIDtoTIME[repoName] h.AssertEq(t, exists, false) @@ -507,7 +515,7 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, err) h.AssertNil(t, img.Save()) - mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, l logging.Logger) (bool, error) { + mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, path string) (bool, error) { return true, nil } @@ -524,12 +532,12 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { }, } - mockImagePullChecker.MockReadImageJSON = func(l logging.Logger) (*image.ImageJSON, error) { + mockImagePullChecker.MockRead = func(path string) (*image.ImageJSON, error) { return imageJSON, nil } - mockImagePullChecker.MockUpdateImagePullRecord = func(l logging.Logger, imageID string, timestamp string) error { - imageJSON, _ = mockImagePullChecker.ReadImageJSON(l) + mockImagePullChecker.MockUpdateImagePullRecord = func(path string, imageID string, timestamp string) error { + imageJSON, _ = mockImagePullChecker.Read("") imageJSON.Image.ImageIDtoTIME[repoName] = timestamp return nil } @@ -539,7 +547,7 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { it.After(func() { mockImagePullChecker.MockCheckImagePullInterval = nil - mockImagePullChecker.MockReadImageJSON = nil + mockImagePullChecker.MockRead = nil mockImagePullChecker.MockUpdateImagePullRecord = nil h.DockerRmi(docker, repoName) }) @@ -550,7 +558,7 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { _, err := imageFetcher.Fetch(context.TODO(), repoName, image.FetchOptions{Daemon: true, PullPolicy: image.PullWeekly}) h.AssertNil(t, err) - imageJSON, _ = mockImagePullChecker.ReadImageJSON(logger) + imageJSON, _ = mockImagePullChecker.Read("") afterFetch, _ := time.Parse(time.RFC3339, imageJSON.Image.ImageIDtoTIME[repoName]) fmt.Printf("after fetch: %v\n", imageJSON.Image.ImageIDtoTIME[repoName]) @@ -566,7 +574,7 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, localImg.SetLabel(label, "2")) h.AssertNil(t, localImg.Save()) - mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, l logging.Logger) (bool, error) { + mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, path string) (bool, error) { return true, nil } imageFetcher = image.NewFetcher(logging.NewLogWithWriters(&outBuf, &outBuf), docker, mockImagePullChecker) @@ -593,7 +601,7 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { when("there is no local image and CheckImagePullInterval returns true", func() { it.Before(func() { - mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, l logging.Logger) (bool, error) { + mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, path string) (bool, error) { return true, nil } imageFetcher = image.NewFetcher(logging.NewLogWithWriters(&outBuf, &outBuf), docker, mockImagePullChecker) @@ -611,7 +619,7 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { when("there is no local image and CheckImagePullInterval returns false", func() { it.Before(func() { - mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, l logging.Logger) (bool, error) { + mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, path string) (bool, error) { return false, nil } @@ -629,20 +637,20 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { } imageFetcher = image.NewFetcher(logging.NewLogWithWriters(&outBuf, &outBuf), docker, mockImagePullChecker) - mockImagePullChecker.MockReadImageJSON = func(l logging.Logger) (*image.ImageJSON, error) { + mockImagePullChecker.MockRead = func(path string) (*image.ImageJSON, error) { return imageJSON, nil } }) it.After(func() { mockImagePullChecker.MockCheckImagePullInterval = nil - mockImagePullChecker.MockReadImageJSON = nil + mockImagePullChecker.MockRead = nil }) it("returns an error and deletes the image record", func() { _, err := imageFetcher.Fetch(context.TODO(), repoName, image.FetchOptions{Daemon: true, PullPolicy: image.PullWeekly}) h.AssertError(t, err, fmt.Sprintf("image '%s' does not exist on the daemon", repoName)) - imageJSON, err = mockImagePullChecker.ReadImageJSON(logger) + imageJSON, err = mockImagePullChecker.Read("") h.AssertNil(t, err) _, exists := imageJSON.Image.ImageIDtoTIME[repoName] h.AssertEq(t, exists, false) @@ -656,7 +664,7 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, localImg.SetLabel(label, "2")) h.AssertNil(t, localImg.Save()) - mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, l logging.Logger) (bool, error) { + mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, path string) (bool, error) { return true, nil } imageFetcher = image.NewFetcher(logging.NewLogWithWriters(&outBuf, &outBuf), docker, mockImagePullChecker) @@ -684,7 +692,7 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, localImg.SetLabel(label, "2")) h.AssertNil(t, localImg.Save()) - mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, l logging.Logger) (bool, error) { + mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, path string) (bool, error) { return true, nil } imageFetcher = image.NewFetcher(logging.NewLogWithWriters(&outBuf, &outBuf), docker, mockImagePullChecker) diff --git a/pkg/image/pull_policy.go b/pkg/image/pull_policy.go index 0322ee0e7e..bca560b85b 100644 --- a/pkg/image/pull_policy.go +++ b/pkg/image/pull_policy.go @@ -4,7 +4,6 @@ import ( "encoding/json" "fmt" "os" - "os/exec" "path/filepath" "regexp" "strconv" @@ -13,6 +12,7 @@ import ( "github.com/pkg/errors" + "github.com/buildpacks/pack/internal/config" "github.com/buildpacks/pack/pkg/logging" ) @@ -21,12 +21,15 @@ type PullPolicy int var interval string +type ImagePullPolicyManager struct { + Logger logging.Logger +} + var ( hourly = "1h" daily = "1d" weekly = "7d" intervalRegex = regexp.MustCompile(`^(\d+d)?(\d+h)?(\d+m)?$`) - imagePath string ) const ( @@ -61,29 +64,40 @@ type ImageJSON struct { Image *ImageData `json:"image"` } +func DefaultImageJSONPath() (string, error) { + home, err := config.PackHome() + if err != nil { + return "", errors.Wrap(err, "getting pack home") + } + return filepath.Join(home, "image.json"), nil +} + var nameMap = map[string]PullPolicy{"always": PullAlways, "hourly": PullHourly, "daily": PullDaily, "weekly": PullWeekly, "never": PullNever, "if-not-present": PullIfNotPresent, "": PullAlways} // ParsePullPolicy from string with support for interval formats -func ParsePullPolicy(policy string) (PullPolicy, error) { +func ParsePullPolicy(policy string, logger logging.Logger) (PullPolicy, error) { + pullPolicyManager := NewPullPolicyManager(logger) + if val, ok := nameMap[policy]; ok { if val == PullHourly { - err := updateImageJSONDuration(hourly) + err := pullPolicyManager.updateImageJSONDuration(hourly) if err != nil { return PullAlways, err } } if val == PullDaily { - err := updateImageJSONDuration(daily) + err := pullPolicyManager.updateImageJSONDuration(daily) if err != nil { return PullAlways, err } } if val == PullWeekly { - err := updateImageJSONDuration(weekly) + err := pullPolicyManager.updateImageJSONDuration(weekly) if err != nil { return PullAlways, err } } + return val, nil } @@ -95,7 +109,7 @@ func ParsePullPolicy(policy string) (PullPolicy, error) { return PullAlways, errors.Errorf("invalid interval format: %s", intervalStr) } - err := updateImageJSONDuration(intervalStr) + err := pullPolicyManager.updateImageJSONDuration(intervalStr) if err != nil { return PullAlways, err } @@ -127,60 +141,20 @@ func (p PullPolicy) String() string { return "" } -func updateImageJSONDuration(intervalStr string) error { - imageJSON, err := ReadImageJSON(logging.NewSimpleLogger(os.Stderr)) +func (i *ImagePullPolicyManager) updateImageJSONDuration(intervalStr string) error { + path, err := DefaultImageJSONPath() if err != nil { return err } - imageJSON.Interval.PullingInterval = intervalStr - - updatedJSON, err := json.MarshalIndent(imageJSON, "", " ") + imageJSON, err := i.Read(path) if err != nil { - return errors.Wrap(err, "failed to marshal updated records") - } - - return WriteFile(updatedJSON) -} - -func ReadImageJSON(l logging.Logger) (*ImageJSON, error) { - homeDir, err := os.UserHomeDir() - if err != nil { - return &ImageJSON{}, errors.Wrap(err, "failed to get home directory") - } - imagePath = filepath.Join(homeDir, ".pack", "image.json") - - // Check if the directory exists, if not, create it - dirPath := filepath.Dir(imagePath) - if _, err := os.Stat(dirPath); os.IsNotExist(err) { - l.Warnf("missing `.pack` directory under %s directory %s", homeDir, err) - l.Debugf("creating `.pack` directory under %s directory", homeDir) - if err := os.MkdirAll(dirPath, 0755); err != nil { - return &ImageJSON{}, errors.Wrap(err, "failed to create directory") - } - } - - // Check if the file exists, if not, create it with minimum JSON - if _, err := os.Stat(imagePath); os.IsNotExist(err) { - l.Warnf("missing `image.json` file under %s directory %s", dirPath, err) - l.Debugf("creating `image.json` file under %s directory", dirPath) - minimumJSON := []byte(`{"interval":{"pulling_interval":"","pruning_interval":"7d","last_prune":""},"image":{}}`) - if err := WriteFile(minimumJSON); err != nil { - return &ImageJSON{}, errors.Wrap(err, "failed to create image.json file") - } - } - - jsonData, err := os.ReadFile(imagePath) - if err != nil && !os.IsNotExist(err) { - return &ImageJSON{}, errors.Wrap(err, "failed to read image.json") + return err } - var imageJSON *ImageJSON - if err := json.Unmarshal(jsonData, &imageJSON); err != nil && !os.IsNotExist(err) { - return &ImageJSON{}, errors.Wrap(err, "failed to unmarshal image.json") - } + imageJSON.Interval.PullingInterval = intervalStr - return imageJSON, nil + return i.Write(imageJSON, path) } func parseDurationString(durationStr string) (time.Duration, error) { @@ -214,8 +188,12 @@ func parseDurationString(durationStr string) (time.Duration, error) { return time.Duration(totalMinutes) * time.Minute, nil } -func PruneOldImages(l logging.Logger, f *Fetcher) error { - imageJSON, err := ReadImageJSON(l) +func (i *ImagePullPolicyManager) PruneOldImages(f *Fetcher) error { + path, err := DefaultImageJSONPath() + if err != nil { + return err + } + imageJSON, err := i.Read(path) if err != nil { return err } @@ -263,26 +241,56 @@ func PruneOldImages(l logging.Logger, f *Fetcher) error { imageJSON.Interval.LastPrune = time.Now().Format(time.RFC3339) - updatedJSON, err := json.MarshalIndent(imageJSON, "", " ") - if err != nil { - return errors.Wrap(err, "failed to marshal updated records") - } - - if err := WriteFile(updatedJSON); err != nil { + if err := i.Write(imageJSON, path); err != nil { return errors.Wrap(err, "failed to write updated image.json") } return nil } -func WriteFile(data []byte) error { - cmd := exec.Command("sudo", "sh", "-c", "echo '"+string(data)+"' > "+imagePath) - cmd.Stderr = os.Stderr - cmd.Stdin = os.Stdin +func (i *ImagePullPolicyManager) Read(path string) (*ImageJSON, error) { + // Check if the file exists, if not, return default values + if _, err := os.Stat(path); os.IsNotExist(err) { + return &ImageJSON{ + Interval: &Interval{ + PullingInterval: "7d", + PruningInterval: "7d", + LastPrune: "", + }, + Image: &ImageData{}, + }, nil + } - if err := cmd.Run(); err != nil { - return errors.New("failed to write file with sudo: " + err.Error()) + jsonData, err := os.ReadFile(path) + if err != nil && !os.IsNotExist(err) { + return nil, errors.Wrap(err, "failed to read image.json") + } + var imageJSON *ImageJSON + if err := json.Unmarshal(jsonData, &imageJSON); err != nil && !os.IsNotExist(err) { + return nil, errors.Wrap(err, "failed to unmarshal image.json") + } + return imageJSON, nil +} + +func (i *ImagePullPolicyManager) Write(imageJSON *ImageJSON, path string) error { + updatedJSON, err := json.MarshalIndent(imageJSON, "", " ") + if err != nil { + return errors.Wrap(err, "failed to marshal updated records") } + return WriteFile(updatedJSON, path) +} + +func WriteFile(data []byte, path string) error { + file, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0644) + if err != nil { + return errors.New("failed to open file: " + err.Error()) + } + defer file.Close() + + _, err = file.Write(data) + if err != nil { + return errors.New("failed to write data to file: " + err.Error()) + } return nil } diff --git a/pkg/image/pull_policy_test.go b/pkg/image/pull_policy_test.go index 246cf05eb6..41fde7fddf 100644 --- a/pkg/image/pull_policy_test.go +++ b/pkg/image/pull_policy_test.go @@ -17,54 +17,54 @@ func TestPullPolicy(t *testing.T) { func testPullPolicy(t *testing.T, when spec.G, it spec.S) { when("#ParsePullPolicy", func() { it("returns PullNever for never", func() { - policy, err := image.ParsePullPolicy("never") + policy, err := image.ParsePullPolicy("never", logger) h.AssertNil(t, err) h.AssertEq(t, policy, image.PullNever) }) it("returns PullAlways for always", func() { - policy, err := image.ParsePullPolicy("always") + policy, err := image.ParsePullPolicy("always", logger) h.AssertNil(t, err) h.AssertEq(t, policy, image.PullAlways) }) it("returns PullIfNotPresent for if-not-present", func() { - policy, err := image.ParsePullPolicy("if-not-present") + policy, err := image.ParsePullPolicy("if-not-present", logger) h.AssertNil(t, err) h.AssertEq(t, policy, image.PullIfNotPresent) }) it("returns PullHourly for hourly", func() { - policy, err := image.ParsePullPolicy("hourly") + policy, err := image.ParsePullPolicy("hourly", logger) h.AssertNil(t, err) h.AssertEq(t, policy, image.PullHourly) }) it("returns PullDaily for daily", func() { - policy, err := image.ParsePullPolicy("daily") + policy, err := image.ParsePullPolicy("daily", logger) h.AssertNil(t, err) h.AssertEq(t, policy, image.PullDaily) }) it("returns PullWeekly for weekly", func() { - policy, err := image.ParsePullPolicy("weekly") + policy, err := image.ParsePullPolicy("weekly", logger) h.AssertNil(t, err) h.AssertEq(t, policy, image.PullWeekly) }) it("returns PullWithInterval for interval= format", func() { - policy, err := image.ParsePullPolicy("interval=4d") + policy, err := image.ParsePullPolicy("interval=4d", logger) h.AssertNil(t, err) h.AssertEq(t, policy, image.PullWithInterval) }) it("returns error for unknown string", func() { - _, err := image.ParsePullPolicy("fake-policy-here") + _, err := image.ParsePullPolicy("fake-policy-here", logger) h.AssertError(t, err, "invalid pull policy") }) it("returns error for invalid interval format", func() { - _, err := image.ParsePullPolicy("interval=invalid") + _, err := image.ParsePullPolicy("interval=invalid", logger) h.AssertError(t, err, "invalid interval format") }) }) From 59884f8c4c62bd8f8dc39221eb8212b01594faeb Mon Sep 17 00:00:00 2001 From: Parthiba-Hazra Date: Wed, 13 Mar 2024 20:47:26 +0530 Subject: [PATCH 10/11] Refactor image package for better modularity and dependency management - remove circular dependency between `ImagePullChecker` and `Fetcher` in the method PruneOldImages. Moved the creation of the `ImagePullPolicyManager` to the cmd package that reduces the number of instances created. Additionally, renamed the `ImagePullChecker` interface to `ImagePullPolicyHandler` Signed-off-by: Parthiba-Hazra --- cmd/cmd.go | 23 +-- go.mod | 10 +- go.sum | 20 +-- internal/commands/build.go | 4 +- internal/commands/build_test.go | 27 ++-- internal/commands/builder.go | 5 +- internal/commands/builder_create.go | 4 +- internal/commands/builder_create_test.go | 24 +-- internal/commands/builder_test.go | 12 +- internal/commands/buildpack.go | 5 +- internal/commands/buildpack_package.go | 4 +- internal/commands/buildpack_package_test.go | 5 +- internal/commands/buildpack_test.go | 14 +- internal/commands/config.go | 7 +- internal/commands/config_prune_interval.go | 13 +- .../commands/config_prune_interval_test.go | 80 +++++++++- internal/commands/config_pull_policy.go | 8 +- internal/commands/config_pull_policy_test.go | 55 ++++--- internal/commands/config_test.go | 18 ++- internal/commands/create_builder.go | 4 +- internal/commands/create_builder_test.go | 24 +-- internal/commands/extension.go | 5 +- internal/commands/extension_package.go | 4 +- internal/commands/extension_package_test.go | 5 +- internal/commands/extension_test.go | 14 +- internal/commands/package_buildpack.go | 4 +- internal/commands/package_buildpack_test.go | 25 ++- internal/commands/rebase.go | 4 +- internal/commands/rebase_test.go | 27 ++-- internal/commands/testdata/example_image.json | 13 ++ pkg/client/client.go | 26 +-- pkg/image/fetcher.go | 16 +- pkg/image/fetcher_test.go | 148 +++++------------- pkg/image/pull_policy.go | 25 +-- pkg/image/pull_policy_test.go | 32 ++-- pkg/testmocks/mock_image_fetcher.go | 78 +++++++++ 36 files changed, 489 insertions(+), 303 deletions(-) create mode 100644 internal/commands/testdata/example_image.json diff --git a/cmd/cmd.go b/cmd/cmd.go index 837c9c96f4..7108b81afd 100644 --- a/cmd/cmd.go +++ b/cmd/cmd.go @@ -12,6 +12,7 @@ import ( imagewriter "github.com/buildpacks/pack/internal/inspectimage/writer" "github.com/buildpacks/pack/internal/term" "github.com/buildpacks/pack/pkg/client" + "github.com/buildpacks/pack/pkg/image" "github.com/buildpacks/pack/pkg/logging" ) @@ -33,7 +34,9 @@ func NewPackCommand(logger ConfigurableLogger) (*cobra.Command, error) { return nil, err } - packClient, err := initClient(logger, cfg) + imagePullPolicyHandler := image.NewPullPolicyManager(logger) + + packClient, err := initClient(logger, cfg, imagePullPolicyHandler) if err != nil { return nil, err } @@ -75,14 +78,14 @@ func NewPackCommand(logger ConfigurableLogger) (*cobra.Command, error) { commands.AddHelpFlag(rootCmd, "pack") - rootCmd.AddCommand(commands.Build(logger, cfg, packClient)) - rootCmd.AddCommand(commands.NewBuilderCommand(logger, cfg, packClient)) - rootCmd.AddCommand(commands.NewBuildpackCommand(logger, cfg, packClient, buildpackage.NewConfigReader())) - rootCmd.AddCommand(commands.NewExtensionCommand(logger, cfg, packClient, buildpackage.NewConfigReader())) - rootCmd.AddCommand(commands.NewConfigCommand(logger, cfg, cfgPath, packClient)) + rootCmd.AddCommand(commands.Build(logger, cfg, packClient, imagePullPolicyHandler)) + rootCmd.AddCommand(commands.NewBuilderCommand(logger, cfg, packClient, imagePullPolicyHandler)) + rootCmd.AddCommand(commands.NewBuildpackCommand(logger, cfg, packClient, buildpackage.NewConfigReader(), imagePullPolicyHandler)) + rootCmd.AddCommand(commands.NewExtensionCommand(logger, cfg, packClient, buildpackage.NewConfigReader(), imagePullPolicyHandler)) + rootCmd.AddCommand(commands.NewConfigCommand(logger, cfg, cfgPath, packClient, imagePullPolicyHandler)) rootCmd.AddCommand(commands.InspectImage(logger, imagewriter.NewFactory(), cfg, packClient)) rootCmd.AddCommand(commands.NewStackCommand(logger)) - rootCmd.AddCommand(commands.Rebase(logger, cfg, packClient)) + rootCmd.AddCommand(commands.Rebase(logger, cfg, packClient, imagePullPolicyHandler)) rootCmd.AddCommand(commands.NewSBOMCommand(logger, cfg, packClient)) rootCmd.AddCommand(commands.InspectBuildpack(logger, cfg, packClient)) @@ -94,8 +97,8 @@ func NewPackCommand(logger ConfigurableLogger) (*cobra.Command, error) { rootCmd.AddCommand(commands.TrustBuilder(logger, cfg, cfgPath)) rootCmd.AddCommand(commands.UntrustBuilder(logger, cfg, cfgPath)) rootCmd.AddCommand(commands.ListTrustedBuilders(logger, cfg)) - rootCmd.AddCommand(commands.CreateBuilder(logger, cfg, packClient)) - rootCmd.AddCommand(commands.PackageBuildpack(logger, cfg, packClient, buildpackage.NewConfigReader())) + rootCmd.AddCommand(commands.CreateBuilder(logger, cfg, packClient, imagePullPolicyHandler)) + rootCmd.AddCommand(commands.PackageBuildpack(logger, cfg, packClient, buildpackage.NewConfigReader(), imagePullPolicyHandler)) if cfg.Experimental { rootCmd.AddCommand(commands.AddBuildpackRegistry(logger, cfg, cfgPath)) @@ -145,5 +148,5 @@ func initClient(logger logging.Logger, cfg config.Config) (*client.Client, error if err != nil { return nil, err } - return client.NewClient(client.WithLogger(logger), client.WithExperimental(cfg.Experimental), client.WithRegistryMirrors(cfg.RegistryMirrors), client.WithDockerClient(dc)) + return client.NewClient(client.WithLogger(logger), client.WithExperimental(cfg.Experimental), client.WithRegistryMirrors(cfg.RegistryMirrors), client.WithDockerClient(dc), client.WithImagePullChecker(imagePullPolicyHandler)) } diff --git a/go.mod b/go.mod index d89e014a75..019c2e0264 100644 --- a/go.mod +++ b/go.mod @@ -11,7 +11,7 @@ require ( github.com/docker/docker v25.0.5+incompatible github.com/docker/go-connections v0.5.0 github.com/dustin/go-humanize v1.0.1 - github.com/gdamore/tcell/v2 v2.7.1 + github.com/gdamore/tcell/v2 v2.7.4 github.com/go-git/go-git/v5 v5.11.0 github.com/golang/mock v1.6.0 github.com/google/go-cmp v0.6.0 @@ -29,9 +29,9 @@ require ( github.com/sabhiram/go-gitignore v0.0.0-20210923224102-525f6e181f06 github.com/sclevine/spec v1.4.0 github.com/spf13/cobra v1.8.0 - golang.org/x/crypto v0.20.0 - golang.org/x/mod v0.15.0 - golang.org/x/oauth2 v0.17.0 + golang.org/x/crypto v0.21.0 + golang.org/x/mod v0.16.0 + golang.org/x/oauth2 v0.18.0 golang.org/x/sync v0.6.0 golang.org/x/sys v0.18.0 golang.org/x/term v0.18.0 @@ -132,7 +132,7 @@ require ( go.opentelemetry.io/otel v1.22.0 // indirect go.opentelemetry.io/otel/metric v1.22.0 // indirect go.opentelemetry.io/otel/trace v1.22.0 // indirect - golang.org/x/net v0.21.0 // indirect + golang.org/x/net v0.22.0 // indirect golang.org/x/tools v0.18.0 // indirect google.golang.org/appengine v1.6.8 // indirect google.golang.org/protobuf v1.33.0 // indirect diff --git a/go.sum b/go.sum index 17ca08fc80..b78983320a 100644 --- a/go.sum +++ b/go.sum @@ -154,8 +154,8 @@ github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMo github.com/gdamore/encoding v1.0.0 h1:+7OoQ1Bc6eTm5niUzBa0Ctsh6JbMW6Ra+YNuAtDBdko= github.com/gdamore/encoding v1.0.0/go.mod h1:alR0ol34c49FCSBLjhosxzcPHQbf2trDkoo5dl+VrEg= github.com/gdamore/tcell/v2 v2.4.1-0.20210905002822-f057f0a857a1/go.mod h1:Az6Jt+M5idSED2YPGtwnfJV0kXohgdCBPmHGSYc1r04= -github.com/gdamore/tcell/v2 v2.7.1 h1:TiCcmpWHiAU7F0rA2I3S2Y4mmLmO9KHxJ7E1QhYzQbc= -github.com/gdamore/tcell/v2 v2.7.1/go.mod h1:dSXtXTSK0VsW1biw65DZLZ2NKr7j0qP/0J7ONmsraWg= +github.com/gdamore/tcell/v2 v2.7.4 h1:sg6/UnTM9jGpZU+oFYAsDahfchWAFW8Xx2yFinNSAYU= +github.com/gdamore/tcell/v2 v2.7.4/go.mod h1:dSXtXTSK0VsW1biw65DZLZ2NKr7j0qP/0J7ONmsraWg= github.com/gliderlabs/ssh v0.3.5 h1:OcaySEmAQJgyYcArR+gGGTHCyE7nvhEMTlYY+Dp8CpY= github.com/gliderlabs/ssh v0.3.5/go.mod h1:8XB4KraRrX39qHhT6yxPsHedjA08I/uBVwj4xC+/+z4= github.com/go-git/gcfg v1.5.1-0.20230307220236-3a3c6141e376 h1:+zs/tPmkDkHx3U66DAb0lQFJrpS6731Oaa12ikc+DiI= @@ -417,15 +417,15 @@ golang.org/x/crypto v0.0.0-20220722155217-630584e8d5aa/go.mod h1:IxCIyHEi3zRg3s0 golang.org/x/crypto v0.3.1-0.20221117191849-2c476679df9a/go.mod h1:hebNnKkNXi2UzZN1eVRvBB7co0a+JxK6XbPiWVs/3J4= golang.org/x/crypto v0.6.0/go.mod h1:OFC/31mSvZgRz0V1QTNCzfAI1aIRzbiufJtkMIlEp58= golang.org/x/crypto v0.7.0/go.mod h1:pYwdfH91IfpZVANVyUOhSIPZaFoJGxTFbZhFTx+dXZU= -golang.org/x/crypto v0.20.0 h1:jmAMJJZXr5KiCw05dfYK9QnqaqKLYXijU23lsEdcQqg= -golang.org/x/crypto v0.20.0/go.mod h1:Xwo95rrVNIoSMx9wa1JroENMToLWn3RNVrTBpLHgZPQ= +golang.org/x/crypto v0.21.0 h1:X31++rzVUdKhX5sWmSOFZxx8UW/ldWx55cbf08iNAMA= +golang.org/x/crypto v0.21.0/go.mod h1:0BP7YvVV9gBbVKyeTG0Gyn+gZm94bibOW5BjDEYAOMs= golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= -golang.org/x/mod v0.15.0 h1:SernR4v+D55NyBH2QiEQrlBAnj1ECL6AGrA5+dPaMY8= -golang.org/x/mod v0.15.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c= +golang.org/x/mod v0.16.0 h1:QX4fJ0Rr5cPQCF7O9lh9Se4pmwfwskqZfq5moyldzic= +golang.org/x/mod v0.16.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c= golang.org/x/net v0.0.0-20180906233101-161cd47e91fd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20181114220301-adae6a3d119a/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= @@ -441,11 +441,11 @@ golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug golang.org/x/net v0.2.0/go.mod h1:KqCZLdyyvdV855qA2rE3GC2aiw5xGR5TEjj8smXukLY= golang.org/x/net v0.6.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs= golang.org/x/net v0.8.0/go.mod h1:QVkue5JL9kW//ek3r6jTKnTFis1tRmNAW2P1shuFdJc= -golang.org/x/net v0.21.0 h1:AQyQV4dYCvJ7vGmJyKki9+PBdyvhkSd8EIx/qb0AYv4= -golang.org/x/net v0.21.0/go.mod h1:bIjVDfnllIU7BJ2DNgfnXvpSvtn8VRwhlsaeUTyUS44= +golang.org/x/net v0.22.0 h1:9sGLhx7iRIHEiX0oAJ3MRZMUCElJgy7Br1nO+AMN3Tc= +golang.org/x/net v0.22.0/go.mod h1:JKghWKKOSdJwpW2GEx0Ja7fmaKnMsbu+MWVZTokSYmg= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= -golang.org/x/oauth2 v0.17.0 h1:6m3ZPmLEFdVxKKWnKq4VqZ60gutO35zm+zrAHVmHyDQ= -golang.org/x/oauth2 v0.17.0/go.mod h1:OzPDGQiuQMguemayvdylqddI7qcD9lnSDb+1FiwQ5HA= +golang.org/x/oauth2 v0.18.0 h1:09qnuIAgzdx1XplqJvW6CQqMCtGZykZWcXzPMPUusvI= +golang.org/x/oauth2 v0.18.0/go.mod h1:Wf7knwG0MPoWIMMBgFlEaSUDaKskp0dCfrlJRJXbBi8= golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= diff --git a/internal/commands/build.go b/internal/commands/build.go index d04c74ab54..5f3b9f302a 100644 --- a/internal/commands/build.go +++ b/internal/commands/build.go @@ -58,7 +58,7 @@ type BuildFlags struct { } // Build an image from source code -func Build(logger logging.Logger, cfg config.Config, packClient PackClient) *cobra.Command { +func Build(logger logging.Logger, cfg config.Config, packClient PackClient, imagePullPolicyHandler image.ImagePullPolicyHandler) *cobra.Command { var flags BuildFlags cmd := &cobra.Command{ @@ -128,7 +128,7 @@ func Build(logger logging.Logger, cfg config.Config, packClient PackClient) *cob if stringPolicy == "" { stringPolicy = cfg.PullPolicy } - pullPolicy, err := image.ParsePullPolicy(stringPolicy, logger) + pullPolicy, err := imagePullPolicyHandler.ParsePullPolicy(stringPolicy) if err != nil { return errors.Wrapf(err, "parsing pull policy %s", flags.Policy) } diff --git a/internal/commands/build_test.go b/internal/commands/build_test.go index 6a59810004..3644f70a83 100644 --- a/internal/commands/build_test.go +++ b/internal/commands/build_test.go @@ -26,6 +26,7 @@ import ( "github.com/buildpacks/pack/pkg/image" "github.com/buildpacks/pack/pkg/logging" projectTypes "github.com/buildpacks/pack/pkg/project/types" + fetcher_mock "github.com/buildpacks/pack/pkg/testmocks" h "github.com/buildpacks/pack/testhelpers" ) @@ -38,12 +39,13 @@ func TestBuildCommand(t *testing.T) { func testBuildCommand(t *testing.T, when spec.G, it spec.S) { var ( - command *cobra.Command - logger *logging.LogWithWriters - outBuf bytes.Buffer - mockController *gomock.Controller - mockClient *testmocks.MockPackClient - cfg config.Config + command *cobra.Command + logger *logging.LogWithWriters + outBuf bytes.Buffer + mockController *gomock.Controller + mockClient *testmocks.MockPackClient + cfg config.Config + imagePullPolicyHandler image.ImagePullPolicyHandler ) it.Before(func() { @@ -51,8 +53,9 @@ func testBuildCommand(t *testing.T, when spec.G, it spec.S) { cfg = config.Config{} mockController = gomock.NewController(t) mockClient = testmocks.NewMockPackClient(mockController) + imagePullPolicyHandler = fetcher_mock.NewMockPullPolicyManager(logger) - command = commands.Build(logger, cfg, mockClient) + command = commands.Build(logger, cfg, mockClient, imagePullPolicyHandler) }) when("#BuildCommand", func() { @@ -97,7 +100,7 @@ func testBuildCommand(t *testing.T, when spec.G, it spec.S) { Return(nil) cfg := config.Config{TrustedBuilders: []config.TrustedBuilder{{Name: "my-builder"}}} - command = commands.Build(logger, cfg, mockClient) + command = commands.Build(logger, cfg, mockClient, imagePullPolicyHandler) }) it("sets the trust builder option", func() { logger.WantVerbose(true) @@ -167,7 +170,7 @@ func testBuildCommand(t *testing.T, when spec.G, it spec.S) { Return(nil) cfg := config.Config{PullPolicy: "if-not-present"} - command := commands.Build(logger, cfg, mockClient) + command := commands.Build(logger, cfg, mockClient, imagePullPolicyHandler) logger.WantVerbose(true) command.SetArgs([]string{"image", "--builder", "my-builder", "--pull-policy", "never"}) @@ -193,7 +196,7 @@ func testBuildCommand(t *testing.T, when spec.G, it spec.S) { Return(nil) cfg := config.Config{PullPolicy: "never"} - command := commands.Build(logger, cfg, mockClient) + command := commands.Build(logger, cfg, mockClient, imagePullPolicyHandler) logger.WantVerbose(true) command.SetArgs([]string{"image", "--builder", "my-builder"}) @@ -455,7 +458,7 @@ func testBuildCommand(t *testing.T, when spec.G, it spec.S) { Return(nil) cfg := config.Config{LifecycleImage: "some-lifecycle-image"} - command := commands.Build(logger, cfg, mockClient) + command := commands.Build(logger, cfg, mockClient, imagePullPolicyHandler) logger.WantVerbose(true) command.SetArgs([]string{"image", "--builder", "my-builder"}) @@ -895,7 +898,7 @@ builder = "my-builder" Experimental: true, LayoutRepositoryDir: layoutDir, } - command = commands.Build(logger, cfg, mockClient) + command = commands.Build(logger, cfg, mockClient, imagePullPolicyHandler) }) when("path to save the image is provided", func() { diff --git a/internal/commands/builder.go b/internal/commands/builder.go index 50d5efdd1a..c226a597a5 100644 --- a/internal/commands/builder.go +++ b/internal/commands/builder.go @@ -5,10 +5,11 @@ import ( builderwriter "github.com/buildpacks/pack/internal/builder/writer" "github.com/buildpacks/pack/internal/config" + "github.com/buildpacks/pack/pkg/image" "github.com/buildpacks/pack/pkg/logging" ) -func NewBuilderCommand(logger logging.Logger, cfg config.Config, client PackClient) *cobra.Command { +func NewBuilderCommand(logger logging.Logger, cfg config.Config, client PackClient, imagePullPolicyHandler image.ImagePullPolicyHandler) *cobra.Command { cmd := &cobra.Command{ Use: "builder", Aliases: []string{"builders"}, @@ -16,7 +17,7 @@ func NewBuilderCommand(logger logging.Logger, cfg config.Config, client PackClie RunE: nil, } - cmd.AddCommand(BuilderCreate(logger, cfg, client)) + cmd.AddCommand(BuilderCreate(logger, cfg, client, imagePullPolicyHandler)) cmd.AddCommand(BuilderInspect(logger, cfg, client, builderwriter.NewFactory())) cmd.AddCommand(BuilderSuggest(logger, client)) AddHelpFlag(cmd, "builder") diff --git a/internal/commands/builder_create.go b/internal/commands/builder_create.go index d6604e3576..3e9ddd3388 100644 --- a/internal/commands/builder_create.go +++ b/internal/commands/builder_create.go @@ -27,7 +27,7 @@ type BuilderCreateFlags struct { } // CreateBuilder creates a builder image, based on a builder config -func BuilderCreate(logger logging.Logger, cfg config.Config, pack PackClient) *cobra.Command { +func BuilderCreate(logger logging.Logger, cfg config.Config, pack PackClient, imagePullPolicyHandler image.ImagePullPolicyHandler) *cobra.Command { var flags BuilderCreateFlags cmd := &cobra.Command{ @@ -50,7 +50,7 @@ Creating a custom builder allows you to control what buildpacks are used and wha if stringPolicy == "" { stringPolicy = cfg.PullPolicy } - pullPolicy, err := image.ParsePullPolicy(stringPolicy, logger) + pullPolicy, err := imagePullPolicyHandler.ParsePullPolicy(stringPolicy) if err != nil { return errors.Wrapf(err, "parsing pull policy %s", flags.Policy) } diff --git a/internal/commands/builder_create_test.go b/internal/commands/builder_create_test.go index b89e7ab534..97980b69fa 100644 --- a/internal/commands/builder_create_test.go +++ b/internal/commands/builder_create_test.go @@ -17,7 +17,9 @@ import ( "github.com/buildpacks/pack/internal/commands" "github.com/buildpacks/pack/internal/commands/testmocks" "github.com/buildpacks/pack/internal/config" + "github.com/buildpacks/pack/pkg/image" "github.com/buildpacks/pack/pkg/logging" + fetcher_mock "github.com/buildpacks/pack/pkg/testmocks" h "github.com/buildpacks/pack/testhelpers" ) @@ -160,14 +162,15 @@ func TestCreateCommand(t *testing.T) { func testCreateCommand(t *testing.T, when spec.G, it spec.S) { var ( - command *cobra.Command - logger logging.Logger - outBuf bytes.Buffer - mockController *gomock.Controller - mockClient *testmocks.MockPackClient - tmpDir string - builderConfigPath string - cfg config.Config + command *cobra.Command + logger logging.Logger + outBuf bytes.Buffer + mockController *gomock.Controller + mockClient *testmocks.MockPackClient + tmpDir string + builderConfigPath string + cfg config.Config + imagePullPolicyHandler image.ImagePullPolicyHandler ) it.Before(func() { @@ -180,7 +183,8 @@ func testCreateCommand(t *testing.T, when spec.G, it spec.S) { mockController = gomock.NewController(t) mockClient = testmocks.NewMockPackClient(mockController) logger = logging.NewLogWithWriters(&outBuf, &outBuf) - command = commands.BuilderCreate(logger, cfg, mockClient) + imagePullPolicyHandler = fetcher_mock.NewMockPullPolicyManager(logger) + command = commands.BuilderCreate(logger, cfg, mockClient, imagePullPolicyHandler) }) it.After(func() { @@ -218,7 +222,7 @@ func testCreateCommand(t *testing.T, when spec.G, it spec.S) { when("configured pull policy is invalid", func() { it("errors when config set with unknown policy", func() { cfg = config.Config{PullPolicy: "unknown-policy"} - command = commands.BuilderCreate(logger, cfg, mockClient) + command = commands.BuilderCreate(logger, cfg, mockClient, imagePullPolicyHandler) command.SetArgs([]string{ "some/builder", "--config", builderConfigPath, diff --git a/internal/commands/builder_test.go b/internal/commands/builder_test.go index 801bc873d6..8bf41d82f8 100644 --- a/internal/commands/builder_test.go +++ b/internal/commands/builder_test.go @@ -12,7 +12,9 @@ import ( "github.com/buildpacks/pack/internal/commands" "github.com/buildpacks/pack/internal/commands/testmocks" "github.com/buildpacks/pack/internal/config" + "github.com/buildpacks/pack/pkg/image" "github.com/buildpacks/pack/pkg/logging" + fetcher_mock "github.com/buildpacks/pack/pkg/testmocks" h "github.com/buildpacks/pack/testhelpers" ) @@ -22,16 +24,18 @@ func TestBuilderCommand(t *testing.T) { func testBuilderCommand(t *testing.T, when spec.G, it spec.S) { var ( - cmd *cobra.Command - logger logging.Logger - outBuf bytes.Buffer + cmd *cobra.Command + logger logging.Logger + outBuf bytes.Buffer + imagePullPolicyHandler image.ImagePullPolicyHandler ) it.Before(func() { logger = logging.NewLogWithWriters(&outBuf, &outBuf) + imagePullPolicyHandler = fetcher_mock.NewMockPullPolicyManager(logger) mockController := gomock.NewController(t) mockClient := testmocks.NewMockPackClient(mockController) - cmd = commands.NewBuilderCommand(logger, config.Config{}, mockClient) + cmd = commands.NewBuilderCommand(logger, config.Config{}, mockClient, imagePullPolicyHandler) cmd.SetOut(logging.GetWriterForLevel(logger, logging.InfoLevel)) }) diff --git a/internal/commands/buildpack.go b/internal/commands/buildpack.go index 93985be84f..3e5ad5d8f0 100644 --- a/internal/commands/buildpack.go +++ b/internal/commands/buildpack.go @@ -4,10 +4,11 @@ import ( "github.com/spf13/cobra" "github.com/buildpacks/pack/internal/config" + "github.com/buildpacks/pack/pkg/image" "github.com/buildpacks/pack/pkg/logging" ) -func NewBuildpackCommand(logger logging.Logger, cfg config.Config, client PackClient, packageConfigReader PackageConfigReader) *cobra.Command { +func NewBuildpackCommand(logger logging.Logger, cfg config.Config, client PackClient, packageConfigReader PackageConfigReader, imagePullPolicyHandler image.ImagePullPolicyHandler) *cobra.Command { cmd := &cobra.Command{ Use: "buildpack", Aliases: []string{"buildpacks"}, @@ -16,7 +17,7 @@ func NewBuildpackCommand(logger logging.Logger, cfg config.Config, client PackCl } cmd.AddCommand(BuildpackInspect(logger, cfg, client)) - cmd.AddCommand(BuildpackPackage(logger, cfg, client, packageConfigReader)) + cmd.AddCommand(BuildpackPackage(logger, cfg, client, packageConfigReader, imagePullPolicyHandler)) cmd.AddCommand(BuildpackNew(logger, client)) cmd.AddCommand(BuildpackPull(logger, cfg, client)) cmd.AddCommand(BuildpackRegister(logger, cfg, client)) diff --git a/internal/commands/buildpack_package.go b/internal/commands/buildpack_package.go index 35f99ffc98..633be947ba 100644 --- a/internal/commands/buildpack_package.go +++ b/internal/commands/buildpack_package.go @@ -40,7 +40,7 @@ type PackageConfigReader interface { } // BuildpackPackage packages (a) buildpack(s) into OCI format, based on a package config -func BuildpackPackage(logger logging.Logger, cfg config.Config, packager BuildpackPackager, packageConfigReader PackageConfigReader) *cobra.Command { +func BuildpackPackage(logger logging.Logger, cfg config.Config, packager BuildpackPackager, packageConfigReader PackageConfigReader, imagePullPolicyHandler image.ImagePullPolicyHandler) *cobra.Command { var flags BuildpackPackageFlags cmd := &cobra.Command{ Use: "package --config ", @@ -62,7 +62,7 @@ func BuildpackPackage(logger logging.Logger, cfg config.Config, packager Buildpa if stringPolicy == "" { stringPolicy = cfg.PullPolicy } - pullPolicy, err := image.ParsePullPolicy(stringPolicy, logger) + pullPolicy, err := imagePullPolicyHandler.ParsePullPolicy(stringPolicy) if err != nil { return errors.Wrap(err, "parsing pull policy") } diff --git a/internal/commands/buildpack_package_test.go b/internal/commands/buildpack_package_test.go index f527c01d08..641654a1af 100644 --- a/internal/commands/buildpack_package_test.go +++ b/internal/commands/buildpack_package_test.go @@ -19,6 +19,7 @@ import ( "github.com/buildpacks/pack/pkg/dist" "github.com/buildpacks/pack/pkg/image" "github.com/buildpacks/pack/pkg/logging" + fetcher_mock "github.com/buildpacks/pack/pkg/testmocks" h "github.com/buildpacks/pack/testhelpers" ) @@ -355,11 +356,13 @@ func packageCommand(ops ...packageCommandOption) *cobra.Command { configPath: "/path/to/some/file", } + imagePullPolicyHandler := fetcher_mock.NewMockPullPolicyManager(config.logger) + for _, op := range ops { op(config) } - cmd := commands.BuildpackPackage(config.logger, config.clientConfig, config.buildpackPackager, config.packageConfigReader) + cmd := commands.BuildpackPackage(config.logger, config.clientConfig, config.buildpackPackager, config.packageConfigReader, imagePullPolicyHandler) cmd.SetArgs([]string{config.imageName, "--config", config.configPath, "-p", config.path}) return cmd diff --git a/internal/commands/buildpack_test.go b/internal/commands/buildpack_test.go index 761486e494..ad0f479cce 100644 --- a/internal/commands/buildpack_test.go +++ b/internal/commands/buildpack_test.go @@ -13,7 +13,9 @@ import ( "github.com/buildpacks/pack/internal/commands/fakes" "github.com/buildpacks/pack/internal/commands/testmocks" "github.com/buildpacks/pack/internal/config" + "github.com/buildpacks/pack/pkg/image" "github.com/buildpacks/pack/pkg/logging" + fetcher_mock "github.com/buildpacks/pack/pkg/testmocks" h "github.com/buildpacks/pack/testhelpers" ) @@ -23,17 +25,19 @@ func TestBuildpackCommand(t *testing.T) { func testBuildpackCommand(t *testing.T, when spec.G, it spec.S) { var ( - cmd *cobra.Command - logger logging.Logger - outBuf bytes.Buffer - mockClient *testmocks.MockPackClient + cmd *cobra.Command + logger logging.Logger + outBuf bytes.Buffer + mockClient *testmocks.MockPackClient + imagePullPolicyHandler image.ImagePullPolicyHandler ) it.Before(func() { logger = logging.NewLogWithWriters(&outBuf, &outBuf) + imagePullPolicyHandler = fetcher_mock.NewMockPullPolicyManager(logger) mockController := gomock.NewController(t) mockClient = testmocks.NewMockPackClient(mockController) - cmd = commands.NewBuildpackCommand(logger, config.Config{}, mockClient, fakes.NewFakePackageConfigReader()) + cmd = commands.NewBuildpackCommand(logger, config.Config{}, mockClient, fakes.NewFakePackageConfigReader(), imagePullPolicyHandler) cmd.SetOut(logging.GetWriterForLevel(logger, logging.InfoLevel)) }) diff --git a/internal/commands/config.go b/internal/commands/config.go index 0d183bc9be..3073cf3443 100644 --- a/internal/commands/config.go +++ b/internal/commands/config.go @@ -6,10 +6,11 @@ import ( "github.com/spf13/cobra" "github.com/buildpacks/pack/internal/config" + "github.com/buildpacks/pack/pkg/image" "github.com/buildpacks/pack/pkg/logging" ) -func NewConfigCommand(logger logging.Logger, cfg config.Config, cfgPath string, client PackClient) *cobra.Command { +func NewConfigCommand(logger logging.Logger, cfg config.Config, cfgPath string, client PackClient, imagePullPolicyHandler image.ImagePullPolicyHandler) *cobra.Command { cmd := &cobra.Command{ Use: "config", Short: "Interact with your local pack config file", @@ -18,8 +19,8 @@ func NewConfigCommand(logger logging.Logger, cfg config.Config, cfgPath string, cmd.AddCommand(ConfigDefaultBuilder(logger, cfg, cfgPath, client)) cmd.AddCommand(ConfigExperimental(logger, cfg, cfgPath)) - cmd.AddCommand(ConfigPullPolicy(logger, cfg, cfgPath)) - cmd.AddCommand(ConfigPruneInterval(logger, cfg, cfgPath)) + cmd.AddCommand(ConfigPullPolicy(logger, cfg, cfgPath, imagePullPolicyHandler)) + cmd.AddCommand(ConfigPruneInterval(logger, cfg, cfgPath, imagePullPolicyHandler)) cmd.AddCommand(ConfigRegistries(logger, cfg, cfgPath)) cmd.AddCommand(ConfigRunImagesMirrors(logger, cfg, cfgPath)) cmd.AddCommand(ConfigTrustedBuilder(logger, cfg, cfgPath)) diff --git a/internal/commands/config_prune_interval.go b/internal/commands/config_prune_interval.go index eae27bb307..5e00527e59 100644 --- a/internal/commands/config_prune_interval.go +++ b/internal/commands/config_prune_interval.go @@ -13,10 +13,9 @@ import ( "github.com/buildpacks/pack/pkg/logging" ) -func ConfigPruneInterval(logger logging.Logger, cfg config.Config, cfgPath string) *cobra.Command { +func ConfigPruneInterval(logger logging.Logger, cfg config.Config, cfgPath string, imagePullPolicyHandler image.ImagePullPolicyHandler) *cobra.Command { var unset bool var intervalRegex = regexp.MustCompile(`^(\d+d)?(\d+h)?(\d+m)?$`) - pullPolicyManager := image.NewPullPolicyManager(logger) cmd := &cobra.Command{ Use: "prune-interval", @@ -38,21 +37,21 @@ func ConfigPruneInterval(logger logging.Logger, cfg config.Config, cfgPath strin if len(args) > 0 { return errors.Errorf("prune interval and --unset cannot be specified simultaneously") } - imageJSON, err := pullPolicyManager.Read(imageJSONPath) + imageJSON, err := imagePullPolicyHandler.Read(imageJSONPath) if err != nil { return err } oldPruneInterval := imageJSON.Interval.PruningInterval imageJSON.Interval.PruningInterval = "7d" - err = pullPolicyManager.Write(imageJSON, imageJSONPath) + err = imagePullPolicyHandler.Write(imageJSON, imageJSONPath) if err != nil { return err } logger.Infof("Successfully unset pruning interval %s", style.Symbol(oldPruneInterval)) logger.Infof("Pruning interval has been set to %s", style.Symbol(imageJSON.Interval.PruningInterval)) case len(args) == 0: // list - imageJSON, err := pullPolicyManager.Read(imageJSONPath) + imageJSON, err := imagePullPolicyHandler.Read(imageJSONPath) if err != nil { return err } @@ -65,7 +64,7 @@ func ConfigPruneInterval(logger logging.Logger, cfg config.Config, cfgPath strin default: // set newPruneInterval := args[0] - imageJSON, err := pullPolicyManager.Read(imageJSONPath) + imageJSON, err := imagePullPolicyHandler.Read(imageJSONPath) if err != nil { return err } @@ -86,7 +85,7 @@ func ConfigPruneInterval(logger logging.Logger, cfg config.Config, cfgPath strin imageJSON.Interval.PruningInterval = newPruneInterval - err = pullPolicyManager.Write(imageJSON, imageJSONPath) + err = imagePullPolicyHandler.Write(imageJSON, imageJSONPath) if err != nil { return err } diff --git a/internal/commands/config_prune_interval_test.go b/internal/commands/config_prune_interval_test.go index 4cedb231dc..0f0d7a4b82 100644 --- a/internal/commands/config_prune_interval_test.go +++ b/internal/commands/config_prune_interval_test.go @@ -12,31 +12,37 @@ import ( "github.com/buildpacks/pack/internal/commands" "github.com/buildpacks/pack/internal/config" + "github.com/buildpacks/pack/pkg/image" "github.com/buildpacks/pack/pkg/logging" + fetcher_mock "github.com/buildpacks/pack/pkg/testmocks" h "github.com/buildpacks/pack/testhelpers" ) +var imageJSON *image.ImageJSON + func TestConfigPruneInterval(t *testing.T) { spec.Run(t, "ConfigPruneIntervalCommand", testConfigPruneIntervalCommand, spec.Random(), spec.Report(report.Terminal{})) } func testConfigPruneIntervalCommand(t *testing.T, when spec.G, it spec.S) { var ( - command *cobra.Command - logger logging.Logger - outBuf bytes.Buffer - tempPackHome string - configFile string - assert = h.NewAssertionManager(t) - cfg = config.Config{} + command *cobra.Command + logger logging.Logger + outBuf bytes.Buffer + tempPackHome string + configFile string + assert = h.NewAssertionManager(t) + cfg = config.Config{} + imagePullPolicyHandler *fetcher_mock.MockImagePullPolicyHandler ) it.Before(func() { logger = logging.NewLogWithWriters(&outBuf, &outBuf) + imagePullPolicyHandler = fetcher_mock.NewMockPullPolicyManager(logger) tempPackHome, _ = os.MkdirTemp("", "pack-home") configFile = filepath.Join(tempPackHome, "config.toml") - command = commands.ConfigPruneInterval(logger, cfg, configFile) + command = commands.ConfigPruneInterval(logger, cfg, configFile, imagePullPolicyHandler) }) it.After(func() { @@ -45,6 +51,21 @@ func testConfigPruneIntervalCommand(t *testing.T, when spec.G, it spec.S) { when("#ConfigPruneInterval", func() { when("no arguments are provided", func() { + it.Before(func() { + interval := "5d" + command.SetArgs([]string{interval}) + + err := command.Execute() + assert.Nil(err) + }) + + it.After(func() { + command.SetArgs([]string{"--unset"}) + + err := command.Execute() + assert.Nil(err) + }) + it("lists the current pruning interval", func() { command.SetArgs([]string{}) @@ -57,6 +78,13 @@ func testConfigPruneIntervalCommand(t *testing.T, when spec.G, it spec.S) { when("an argument is provided", func() { when("argument is valid", func() { + it.After(func() { + command.SetArgs([]string{"--unset"}) + + err := command.Execute() + assert.Nil(err) + }) + it("sets the provided interval as the pruning interval", func() { interval := "5d" command.SetArgs([]string{interval}) @@ -79,6 +107,42 @@ func testConfigPruneIntervalCommand(t *testing.T, when spec.G, it spec.S) { assert.Contains(err.Error(), "invalid interval format") }) }) + + when("argument is valid and the same as the already set pruning interval", func() { + it.Before(func() { + imageJSON = &image.ImageJSON{ + Interval: &image.Interval{ + PullingInterval: "7d", + PruningInterval: "5d", + LastPrune: "2023-01-01T00:00:00Z", + }, + Image: &image.ImageData{ + ImageIDtoTIME: map[string]string{}, + }, + } + + imagePullPolicyHandler.MockRead = func(path string) (*image.ImageJSON, error) { + return imageJSON, nil + } + }) + + it.After(func() { + command.SetArgs([]string{"--unset"}) + + err := command.Execute() + assert.Nil(err) + imagePullPolicyHandler.MockRead = nil + }) + + it("sets the provided interval as the pruning interval", func() { + interval := "5d" + command.SetArgs([]string{interval}) + + err := command.Execute() + assert.Nil(err) + assert.Contains(outBuf.String(), "Prune Interval is already set to") + }) + }) }) when("--unset flag is provided", func() { diff --git a/internal/commands/config_pull_policy.go b/internal/commands/config_pull_policy.go index a5d1968b99..908059b0b1 100644 --- a/internal/commands/config_pull_policy.go +++ b/internal/commands/config_pull_policy.go @@ -12,7 +12,7 @@ import ( "github.com/buildpacks/pack/pkg/logging" ) -func ConfigPullPolicy(logger logging.Logger, cfg config.Config, cfgPath string) *cobra.Command { +func ConfigPullPolicy(logger logging.Logger, cfg config.Config, cfgPath string, imagePullPolicyHandler image.ImagePullPolicyHandler) *cobra.Command { var unset bool cmd := &cobra.Command{ @@ -36,7 +36,7 @@ func ConfigPullPolicy(logger logging.Logger, cfg config.Config, cfgPath string) return errors.Wrapf(err, "writing config to %s", cfgPath) } - pullPolicy, err := image.ParsePullPolicy(cfg.PullPolicy, logger) + pullPolicy, err := imagePullPolicyHandler.ParsePullPolicy(cfg.PullPolicy) if err != nil { return err } @@ -44,7 +44,7 @@ func ConfigPullPolicy(logger logging.Logger, cfg config.Config, cfgPath string) logger.Infof("Successfully unset pull policy %s", style.Symbol(oldPullPolicy)) logger.Infof("Pull policy has been set to %s", style.Symbol(pullPolicy.String())) case len(args) == 0: // list - pullPolicy, err := image.ParsePullPolicy(cfg.PullPolicy, logger) + pullPolicy, err := imagePullPolicyHandler.ParsePullPolicy(cfg.PullPolicy) if err != nil { return err } @@ -58,7 +58,7 @@ func ConfigPullPolicy(logger logging.Logger, cfg config.Config, cfgPath string) return nil } - pullPolicy, err := image.ParsePullPolicy(newPullPolicy, logger) + pullPolicy, err := imagePullPolicyHandler.ParsePullPolicy(newPullPolicy) if err != nil { return err } diff --git a/internal/commands/config_pull_policy_test.go b/internal/commands/config_pull_policy_test.go index b913bdf35b..87e62e7405 100644 --- a/internal/commands/config_pull_policy_test.go +++ b/internal/commands/config_pull_policy_test.go @@ -15,6 +15,7 @@ import ( "github.com/buildpacks/pack/internal/commands" "github.com/buildpacks/pack/internal/config" "github.com/buildpacks/pack/pkg/logging" + fetcher_mock "github.com/buildpacks/pack/pkg/testmocks" h "github.com/buildpacks/pack/testhelpers" ) @@ -26,27 +27,43 @@ func TestConfigPullPolicy(t *testing.T) { func testConfigPullPolicyCommand(t *testing.T, when spec.G, it spec.S) { var ( - command *cobra.Command - logger logging.Logger - outBuf bytes.Buffer - tempPackHome string - configFile string - assert = h.NewAssertionManager(t) - cfg = config.Config{} + command *cobra.Command + logger logging.Logger + outBuf bytes.Buffer + tempPackHome string + configFile string + imageJSONData string + assert = h.NewAssertionManager(t) + cfg = config.Config{} + imagePullPolicyHandler *fetcher_mock.MockImagePullPolicyHandler ) it.Before(func() { var err error logger = logging.NewLogWithWriters(&outBuf, &outBuf) + imagePullPolicyHandler = fetcher_mock.NewMockPullPolicyManager(logger) tempPackHome, err = os.MkdirTemp("", "pack-home") h.AssertNil(t, err) + h.AssertNil(t, os.Setenv("PACK_HOME", tempPackHome)) configFile = filepath.Join(tempPackHome, "config.toml") + jsonFilePath := filepath.Join("testdata", "example_image.json") + data, err := os.ReadFile(jsonFilePath) + h.AssertNil(t, err) + imageJSONData = string(data) + + // Create the .pack directory and image.json file + packDir := filepath.Join(tempPackHome, ".pack") + h.AssertNil(t, os.Mkdir(packDir, 0755)) + + imageJSONFile := filepath.Join(packDir, "image.json") + h.AssertNil(t, os.WriteFile(imageJSONFile, []byte(imageJSONData), 0644)) - command = commands.ConfigPullPolicy(logger, cfg, configFile) + command = commands.ConfigPullPolicy(logger, cfg, configFile, imagePullPolicyHandler) command.SetOut(logging.GetWriterForLevel(logger, logging.InfoLevel)) }) it.After(func() { + h.AssertNil(t, os.Unsetenv("PACK_HOME")) h.AssertNil(t, os.RemoveAll(tempPackHome)) }) @@ -64,7 +81,7 @@ func testConfigPullPolicyCommand(t *testing.T, when spec.G, it spec.S) { when("policy set to always in config", func() { it("lists always as pull policy", func() { cfg.PullPolicy = "always" - command = commands.ConfigPullPolicy(logger, cfg, configFile) + command = commands.ConfigPullPolicy(logger, cfg, configFile, imagePullPolicyHandler) command.SetArgs([]string{}) h.AssertNil(t, command.Execute()) @@ -76,7 +93,7 @@ func testConfigPullPolicyCommand(t *testing.T, when spec.G, it spec.S) { when("policy set to never in config", func() { it("lists never as pull policy", func() { cfg.PullPolicy = "never" - command = commands.ConfigPullPolicy(logger, cfg, configFile) + command = commands.ConfigPullPolicy(logger, cfg, configFile, imagePullPolicyHandler) command.SetArgs([]string{}) h.AssertNil(t, command.Execute()) @@ -88,7 +105,7 @@ func testConfigPullPolicyCommand(t *testing.T, when spec.G, it spec.S) { when("policy set to if-not-present in config", func() { it("lists if-not-present as pull policy", func() { cfg.PullPolicy = "if-not-present" - command = commands.ConfigPullPolicy(logger, cfg, configFile) + command = commands.ConfigPullPolicy(logger, cfg, configFile, imagePullPolicyHandler) command.SetArgs([]string{}) h.AssertNil(t, command.Execute()) @@ -100,7 +117,7 @@ func testConfigPullPolicyCommand(t *testing.T, when spec.G, it spec.S) { when("policy set to hourly in config", func() { it("lists hourly as pull policy", func() { cfg.PullPolicy = "hourly" - command = commands.ConfigPullPolicy(logger, cfg, configFile) + command = commands.ConfigPullPolicy(logger, cfg, configFile, imagePullPolicyHandler) command.SetArgs([]string{}) h.AssertNil(t, command.Execute()) @@ -112,7 +129,7 @@ func testConfigPullPolicyCommand(t *testing.T, when spec.G, it spec.S) { when("policy set to daily in config", func() { it("lists daily as pull policy", func() { cfg.PullPolicy = "daily" - command = commands.ConfigPullPolicy(logger, cfg, configFile) + command = commands.ConfigPullPolicy(logger, cfg, configFile, imagePullPolicyHandler) command.SetArgs([]string{}) h.AssertNil(t, command.Execute()) @@ -124,7 +141,7 @@ func testConfigPullPolicyCommand(t *testing.T, when spec.G, it spec.S) { when("policy set to weekly in config", func() { it("lists weekly as pull policy", func() { cfg.PullPolicy = "weekly" - command = commands.ConfigPullPolicy(logger, cfg, configFile) + command = commands.ConfigPullPolicy(logger, cfg, configFile, imagePullPolicyHandler) command.SetArgs([]string{}) h.AssertNil(t, command.Execute()) @@ -136,7 +153,7 @@ func testConfigPullPolicyCommand(t *testing.T, when spec.G, it spec.S) { when("policy set to interval=1d2h30m in config", func() { it("lists interval=1d2h30m as pull policy", func() { cfg.PullPolicy = "interval=1d2h30m" - command = commands.ConfigPullPolicy(logger, cfg, configFile) + command = commands.ConfigPullPolicy(logger, cfg, configFile, imagePullPolicyHandler) command.SetArgs([]string{}) h.AssertNil(t, command.Execute()) @@ -149,7 +166,7 @@ func testConfigPullPolicyCommand(t *testing.T, when spec.G, it spec.S) { when("policy provided is the same as configured pull policy", func() { it("provides a helpful message", func() { cfg.PullPolicy = "if-not-present" - command = commands.ConfigPullPolicy(logger, cfg, configFile) + command = commands.ConfigPullPolicy(logger, cfg, configFile, imagePullPolicyHandler) command.SetArgs([]string{"if-not-present"}) h.AssertNil(t, command.Execute()) @@ -158,7 +175,7 @@ func testConfigPullPolicyCommand(t *testing.T, when spec.G, it spec.S) { h.AssertEq(t, strings.TrimSpace(output), `Pull policy is already set to 'if-not-present'`) }) it("it does not change the configured policy", func() { - command = commands.ConfigPullPolicy(logger, cfg, configFile) + command = commands.ConfigPullPolicy(logger, cfg, configFile, imagePullPolicyHandler) command.SetArgs([]string{"never"}) assert.Succeeds(command.Execute()) @@ -166,7 +183,7 @@ func testConfigPullPolicyCommand(t *testing.T, when spec.G, it spec.S) { assert.Nil(err) assert.Equal(readCfg.PullPolicy, "never") - command = commands.ConfigPullPolicy(logger, cfg, configFile) + command = commands.ConfigPullPolicy(logger, cfg, configFile, imagePullPolicyHandler) command.SetArgs([]string{"never"}) assert.Succeeds(command.Execute()) @@ -205,7 +222,7 @@ func testConfigPullPolicyCommand(t *testing.T, when spec.G, it spec.S) { when("unset", func() { it("removes set policy and resets to default pull policy", func() { command.SetArgs([]string{"never"}) - command = commands.ConfigPullPolicy(logger, cfg, configFile) + command = commands.ConfigPullPolicy(logger, cfg, configFile, imagePullPolicyHandler) command.SetArgs([]string{"--unset"}) assert.Succeeds(command.Execute()) diff --git a/internal/commands/config_test.go b/internal/commands/config_test.go index 15f1bae8ad..b678586d00 100644 --- a/internal/commands/config_test.go +++ b/internal/commands/config_test.go @@ -15,7 +15,9 @@ import ( "github.com/buildpacks/pack/internal/commands" "github.com/buildpacks/pack/internal/commands/testmocks" "github.com/buildpacks/pack/internal/config" + "github.com/buildpacks/pack/pkg/image" "github.com/buildpacks/pack/pkg/logging" + fetcher_mock "github.com/buildpacks/pack/pkg/testmocks" h "github.com/buildpacks/pack/testhelpers" ) @@ -27,12 +29,13 @@ func TestConfigCommand(t *testing.T) { func testConfigCommand(t *testing.T, when spec.G, it spec.S) { var ( - command *cobra.Command - logger logging.Logger - outBuf bytes.Buffer - tempPackHome string - configPath string - mockClient *testmocks.MockPackClient + command *cobra.Command + logger logging.Logger + outBuf bytes.Buffer + tempPackHome string + configPath string + mockClient *testmocks.MockPackClient + imagePullPolicyHandler image.ImagePullPolicyHandler ) it.Before(func() { @@ -42,11 +45,12 @@ func testConfigCommand(t *testing.T, when spec.G, it spec.S) { mockClient = testmocks.NewMockPackClient(mockController) logger = logging.NewLogWithWriters(&outBuf, &outBuf) + imagePullPolicyHandler = fetcher_mock.NewMockPullPolicyManager(logger) tempPackHome, err = os.MkdirTemp("", "pack-home") h.AssertNil(t, err) configPath = filepath.Join(tempPackHome, "config.toml") - command = commands.NewConfigCommand(logger, config.Config{Experimental: true}, configPath, mockClient) + command = commands.NewConfigCommand(logger, config.Config{Experimental: true}, configPath, mockClient, imagePullPolicyHandler) command.SetOut(logging.GetWriterForLevel(logger, logging.InfoLevel)) }) diff --git a/internal/commands/create_builder.go b/internal/commands/create_builder.go index 4ac3f6a09a..fbca4a3b28 100644 --- a/internal/commands/create_builder.go +++ b/internal/commands/create_builder.go @@ -17,7 +17,7 @@ import ( // Deprecated: Use 'builder create' instead. // CreateBuilder creates a builder image, based on a builder config -func CreateBuilder(logger logging.Logger, cfg config.Config, pack PackClient) *cobra.Command { +func CreateBuilder(logger logging.Logger, cfg config.Config, pack PackClient, imagePullPolicyHandler image.ImagePullPolicyHandler) *cobra.Command { var flags BuilderCreateFlags cmd := &cobra.Command{ @@ -43,7 +43,7 @@ Creating a custom builder allows you to control what buildpacks are used and wha if stringPolicy == "" { stringPolicy = cfg.PullPolicy } - pullPolicy, err := image.ParsePullPolicy(stringPolicy, logger) + pullPolicy, err := imagePullPolicyHandler.ParsePullPolicy(stringPolicy) if err != nil { return errors.Wrapf(err, "parsing pull policy %s", flags.Policy) } diff --git a/internal/commands/create_builder_test.go b/internal/commands/create_builder_test.go index f960ebb2c1..bc1faede57 100644 --- a/internal/commands/create_builder_test.go +++ b/internal/commands/create_builder_test.go @@ -15,7 +15,9 @@ import ( "github.com/buildpacks/pack/internal/commands" "github.com/buildpacks/pack/internal/commands/testmocks" "github.com/buildpacks/pack/internal/config" + "github.com/buildpacks/pack/pkg/image" "github.com/buildpacks/pack/pkg/logging" + fetcher_mock "github.com/buildpacks/pack/pkg/testmocks" h "github.com/buildpacks/pack/testhelpers" ) @@ -27,14 +29,15 @@ func TestCreateBuilderCommand(t *testing.T) { func testCreateBuilderCommand(t *testing.T, when spec.G, it spec.S) { var ( - command *cobra.Command - logger logging.Logger - outBuf bytes.Buffer - mockController *gomock.Controller - mockClient *testmocks.MockPackClient - tmpDir string - builderConfigPath string - cfg config.Config + command *cobra.Command + logger logging.Logger + outBuf bytes.Buffer + mockController *gomock.Controller + mockClient *testmocks.MockPackClient + tmpDir string + builderConfigPath string + cfg config.Config + imagePullPolicyHandler image.ImagePullPolicyHandler ) it.Before(func() { @@ -47,7 +50,8 @@ func testCreateBuilderCommand(t *testing.T, when spec.G, it spec.S) { mockController = gomock.NewController(t) mockClient = testmocks.NewMockPackClient(mockController) logger = logging.NewLogWithWriters(&outBuf, &outBuf) - command = commands.CreateBuilder(logger, cfg, mockClient) + imagePullPolicyHandler = fetcher_mock.NewMockPullPolicyManager(logger) + command = commands.CreateBuilder(logger, cfg, mockClient, imagePullPolicyHandler) }) it.After(func() { @@ -98,7 +102,7 @@ func testCreateBuilderCommand(t *testing.T, when spec.G, it spec.S) { when("configured pull policy is invalid", func() { it("returns error for when config set with unknown policy", func() { cfg = config.Config{PullPolicy: "unknown-policy"} - command = commands.BuilderCreate(logger, cfg, mockClient) + command = commands.BuilderCreate(logger, cfg, mockClient, imagePullPolicyHandler) command.SetArgs([]string{ "some/builder", "--config", builderConfigPath, diff --git a/internal/commands/extension.go b/internal/commands/extension.go index bc4ab36476..54f4840710 100644 --- a/internal/commands/extension.go +++ b/internal/commands/extension.go @@ -4,10 +4,11 @@ import ( "github.com/spf13/cobra" "github.com/buildpacks/pack/internal/config" + "github.com/buildpacks/pack/pkg/image" "github.com/buildpacks/pack/pkg/logging" ) -func NewExtensionCommand(logger logging.Logger, cfg config.Config, client PackClient, packageConfigReader PackageConfigReader) *cobra.Command { +func NewExtensionCommand(logger logging.Logger, cfg config.Config, client PackClient, packageConfigReader PackageConfigReader, imagePullPolicyHandler image.ImagePullPolicyHandler) *cobra.Command { cmd := &cobra.Command{ Use: "extension", Aliases: []string{"extensions"}, @@ -17,7 +18,7 @@ func NewExtensionCommand(logger logging.Logger, cfg config.Config, client PackCl cmd.AddCommand(ExtensionInspect(logger, cfg, client)) // client and packageConfigReader to be passed later on - cmd.AddCommand(ExtensionPackage(logger, cfg, client, packageConfigReader)) + cmd.AddCommand(ExtensionPackage(logger, cfg, client, packageConfigReader, imagePullPolicyHandler)) // client to be passed later on cmd.AddCommand(ExtensionNew(logger)) cmd.AddCommand(ExtensionPull(logger, cfg, client)) diff --git a/internal/commands/extension_package.go b/internal/commands/extension_package.go index cea78a629a..c622867d44 100644 --- a/internal/commands/extension_package.go +++ b/internal/commands/extension_package.go @@ -29,7 +29,7 @@ type ExtensionPackager interface { } // ExtensionPackage packages (a) extension(s) into OCI format, based on a package config -func ExtensionPackage(logger logging.Logger, cfg config.Config, packager ExtensionPackager, packageConfigReader PackageConfigReader) *cobra.Command { +func ExtensionPackage(logger logging.Logger, cfg config.Config, packager ExtensionPackager, packageConfigReader PackageConfigReader, imagePullPolicyHandler image.ImagePullPolicyHandler) *cobra.Command { var flags ExtensionPackageFlags cmd := &cobra.Command{ Use: "package --config ", @@ -45,7 +45,7 @@ func ExtensionPackage(logger logging.Logger, cfg config.Config, packager Extensi stringPolicy = cfg.PullPolicy } - pullPolicy, err := image.ParsePullPolicy(stringPolicy, logger) + pullPolicy, err := imagePullPolicyHandler.ParsePullPolicy(stringPolicy) if err != nil { return errors.Wrap(err, "parsing pull policy") } diff --git a/internal/commands/extension_package_test.go b/internal/commands/extension_package_test.go index a46416e76e..9ed242fe01 100644 --- a/internal/commands/extension_package_test.go +++ b/internal/commands/extension_package_test.go @@ -18,6 +18,7 @@ import ( "github.com/buildpacks/pack/pkg/dist" "github.com/buildpacks/pack/pkg/image" "github.com/buildpacks/pack/pkg/logging" + fetcher_mock "github.com/buildpacks/pack/pkg/testmocks" h "github.com/buildpacks/pack/testhelpers" ) @@ -273,11 +274,13 @@ func packageExtensionCommand(ops ...packageExtensionCommandOption) *cobra.Comman configPath: "/path/to/some/file", } + imagePullPolicyHandler := fetcher_mock.NewMockPullPolicyManager(config.logger) + for _, op := range ops { op(config) } - cmd := commands.ExtensionPackage(config.logger, config.clientConfig, config.extensionPackager, config.packageConfigReader) + cmd := commands.ExtensionPackage(config.logger, config.clientConfig, config.extensionPackager, config.packageConfigReader, imagePullPolicyHandler) cmd.SetArgs([]string{config.imageName, "--config", config.configPath}) return cmd diff --git a/internal/commands/extension_test.go b/internal/commands/extension_test.go index 396d772905..fe2e66dd23 100644 --- a/internal/commands/extension_test.go +++ b/internal/commands/extension_test.go @@ -13,7 +13,9 @@ import ( "github.com/buildpacks/pack/internal/commands/fakes" "github.com/buildpacks/pack/internal/commands/testmocks" "github.com/buildpacks/pack/internal/config" + "github.com/buildpacks/pack/pkg/image" "github.com/buildpacks/pack/pkg/logging" + fetcher_mock "github.com/buildpacks/pack/pkg/testmocks" h "github.com/buildpacks/pack/testhelpers" ) @@ -23,17 +25,19 @@ func TestExtensionCommand(t *testing.T) { func testExtensionCommand(t *testing.T, when spec.G, it spec.S) { var ( - cmd *cobra.Command - logger logging.Logger - outBuf bytes.Buffer - mockClient *testmocks.MockPackClient + cmd *cobra.Command + logger logging.Logger + outBuf bytes.Buffer + mockClient *testmocks.MockPackClient + imagePullPolicyHandler image.ImagePullPolicyHandler ) it.Before(func() { logger = logging.NewLogWithWriters(&outBuf, &outBuf) + imagePullPolicyHandler = fetcher_mock.NewMockPullPolicyManager(logger) mockController := gomock.NewController(t) mockClient = testmocks.NewMockPackClient(mockController) - cmd = commands.NewExtensionCommand(logger, config.Config{}, mockClient, fakes.NewFakePackageConfigReader()) + cmd = commands.NewExtensionCommand(logger, config.Config{}, mockClient, fakes.NewFakePackageConfigReader(), imagePullPolicyHandler) cmd.SetOut(logging.GetWriterForLevel(logger, logging.InfoLevel)) }) diff --git a/internal/commands/package_buildpack.go b/internal/commands/package_buildpack.go index a5fc9b6da9..8757144ec6 100644 --- a/internal/commands/package_buildpack.go +++ b/internal/commands/package_buildpack.go @@ -16,7 +16,7 @@ import ( // Deprecated: use BuildpackPackage instead // PackageBuildpack packages (a) buildpack(s) into OCI format, based on a package config -func PackageBuildpack(logger logging.Logger, cfg config.Config, packager BuildpackPackager, packageConfigReader PackageConfigReader) *cobra.Command { +func PackageBuildpack(logger logging.Logger, cfg config.Config, packager BuildpackPackager, packageConfigReader PackageConfigReader, imagePullPolicyHandler image.ImagePullPolicyHandler) *cobra.Command { var flags BuildpackPackageFlags cmd := &cobra.Command{ @@ -41,7 +41,7 @@ func PackageBuildpack(logger logging.Logger, cfg config.Config, packager Buildpa if stringPolicy == "" { stringPolicy = cfg.PullPolicy } - pullPolicy, err := image.ParsePullPolicy(stringPolicy, logger) + pullPolicy, err := imagePullPolicyHandler.ParsePullPolicy(stringPolicy) if err != nil { return errors.Wrap(err, "parsing pull policy") } diff --git a/internal/commands/package_buildpack_test.go b/internal/commands/package_buildpack_test.go index 585d477edb..dfaeebe1d5 100644 --- a/internal/commands/package_buildpack_test.go +++ b/internal/commands/package_buildpack_test.go @@ -18,6 +18,7 @@ import ( "github.com/buildpacks/pack/pkg/dist" "github.com/buildpacks/pack/pkg/image" "github.com/buildpacks/pack/pkg/logging" + fetcher_mock "github.com/buildpacks/pack/pkg/testmocks" h "github.com/buildpacks/pack/testhelpers" ) @@ -131,11 +132,12 @@ func testPackageBuildpackCommand(t *testing.T, when spec.G, it spec.S) { }) it("takes precedence over a configured pull policy", func() { logger := logging.NewLogWithWriters(&bytes.Buffer{}, &bytes.Buffer{}) + imagePullPolicyHandler := fetcher_mock.NewMockPullPolicyManager(logger) configReader := fakes.NewFakePackageConfigReader() buildpackPackager := &fakes.FakeBuildpackPackager{} clientConfig := config.Config{PullPolicy: "if-not-present"} - command := commands.PackageBuildpack(logger, clientConfig, buildpackPackager, configReader) + command := commands.PackageBuildpack(logger, clientConfig, buildpackPackager, configReader, imagePullPolicyHandler) command.SetArgs([]string{ "some-image-name", "--config", "/path/to/some/file", @@ -153,11 +155,12 @@ func testPackageBuildpackCommand(t *testing.T, when spec.G, it spec.S) { when("configured pull policy", func() { it("uses the configured pull policy", func() { logger := logging.NewLogWithWriters(&bytes.Buffer{}, &bytes.Buffer{}) + imagePullChecker := fetcher_mock.NewMockPullPolicyManager(logger) configReader := fakes.NewFakePackageConfigReader() buildpackPackager := &fakes.FakeBuildpackPackager{} clientConfig := config.Config{PullPolicy: "never"} - command := commands.PackageBuildpack(logger, clientConfig, buildpackPackager, configReader) + command := commands.PackageBuildpack(logger, clientConfig, buildpackPackager, configReader, imagePullChecker) command.SetArgs([]string{ "some-image-name", "--config", "/path/to/some/file", @@ -177,11 +180,12 @@ func testPackageBuildpackCommand(t *testing.T, when spec.G, it spec.S) { when("both --publish and --pull-policy never flags are specified", func() { it("errors with a descriptive message", func() { logger := logging.NewLogWithWriters(&bytes.Buffer{}, &bytes.Buffer{}) + imagePullChecker := fetcher_mock.NewMockPullPolicyManager(logger) configReader := fakes.NewFakePackageConfigReader() buildpackPackager := &fakes.FakeBuildpackPackager{} clientConfig := config.Config{} - command := commands.PackageBuildpack(logger, clientConfig, buildpackPackager, configReader) + command := commands.PackageBuildpack(logger, clientConfig, buildpackPackager, configReader, imagePullChecker) command.SetArgs([]string{ "some-image-name", "--config", "/path/to/some/file", @@ -226,7 +230,9 @@ func testPackageBuildpackCommand(t *testing.T, when spec.G, it spec.S) { configPath: "/path/to/some/file", } - cmd := commands.PackageBuildpack(config.logger, config.clientConfig, config.buildpackPackager, config.packageConfigReader) + imagePullPolicyHandler := fetcher_mock.NewMockPullPolicyManager(config.logger) + + cmd := commands.PackageBuildpack(config.logger, config.clientConfig, config.buildpackPackager, config.packageConfigReader, imagePullPolicyHandler) cmd.SetArgs([]string{config.imageName, "--package-config", config.configPath}) err := cmd.Execute() @@ -244,7 +250,9 @@ func testPackageBuildpackCommand(t *testing.T, when spec.G, it spec.S) { imageName: "some-image-name", } - cmd := commands.PackageBuildpack(config.logger, config.clientConfig, config.buildpackPackager, config.packageConfigReader) + imagePullChecker := fetcher_mock.NewMockPullPolicyManager(config.logger) + + cmd := commands.PackageBuildpack(config.logger, config.clientConfig, config.buildpackPackager, config.packageConfigReader, imagePullChecker) cmd.SetArgs([]string{config.imageName}) err := cmd.Execute() @@ -258,11 +266,12 @@ func testPackageBuildpackCommand(t *testing.T, when spec.G, it spec.S) { when("--pull-policy unknown-policy", func() { it("fails to run", func() { logger := logging.NewLogWithWriters(&bytes.Buffer{}, &bytes.Buffer{}) + imagePullChecker := fetcher_mock.NewMockPullPolicyManager(logger) configReader := fakes.NewFakePackageConfigReader() buildpackPackager := &fakes.FakeBuildpackPackager{} clientConfig := config.Config{} - command := commands.PackageBuildpack(logger, clientConfig, buildpackPackager, configReader) + command := commands.PackageBuildpack(logger, clientConfig, buildpackPackager, configReader, imagePullChecker) command.SetArgs([]string{ "some-image-name", "--config", "/path/to/some/file", @@ -287,11 +296,13 @@ func packageBuildpackCommand(ops ...packageCommandOption) *cobra.Command { configPath: "/path/to/some/file", } + imagePullChecker := fetcher_mock.NewMockPullPolicyManager(config.logger) + for _, op := range ops { op(config) } - cmd := commands.PackageBuildpack(config.logger, config.clientConfig, config.buildpackPackager, config.packageConfigReader) + cmd := commands.PackageBuildpack(config.logger, config.clientConfig, config.buildpackPackager, config.packageConfigReader, imagePullChecker) cmd.SetArgs([]string{config.imageName, "--config", config.configPath}) return cmd diff --git a/internal/commands/rebase.go b/internal/commands/rebase.go index f522163f54..ea61ffff21 100644 --- a/internal/commands/rebase.go +++ b/internal/commands/rebase.go @@ -13,7 +13,7 @@ import ( "github.com/buildpacks/pack/pkg/logging" ) -func Rebase(logger logging.Logger, cfg config.Config, pack PackClient) *cobra.Command { +func Rebase(logger logging.Logger, cfg config.Config, pack PackClient, imagePullPolicyHandler image.ImagePullPolicyHandler) *cobra.Command { var opts client.RebaseOptions var policy string @@ -33,7 +33,7 @@ func Rebase(logger logging.Logger, cfg config.Config, pack PackClient) *cobra.Co if stringPolicy == "" { stringPolicy = cfg.PullPolicy } - opts.PullPolicy, err = image.ParsePullPolicy(stringPolicy, logger) + opts.PullPolicy, err = imagePullPolicyHandler.ParsePullPolicy(stringPolicy) if err != nil { return errors.Wrapf(err, "parsing pull policy %s", stringPolicy) } diff --git a/internal/commands/rebase_test.go b/internal/commands/rebase_test.go index a3bdf05f5e..caf5821b49 100644 --- a/internal/commands/rebase_test.go +++ b/internal/commands/rebase_test.go @@ -18,6 +18,7 @@ import ( "github.com/buildpacks/pack/internal/commands/testmocks" "github.com/buildpacks/pack/internal/config" "github.com/buildpacks/pack/pkg/logging" + fetcher_mock "github.com/buildpacks/pack/pkg/testmocks" h "github.com/buildpacks/pack/testhelpers" ) @@ -30,21 +31,23 @@ func TestRebaseCommand(t *testing.T) { func testRebaseCommand(t *testing.T, when spec.G, it spec.S) { var ( - command *cobra.Command - logger logging.Logger - outBuf bytes.Buffer - mockController *gomock.Controller - mockClient *testmocks.MockPackClient - cfg config.Config + command *cobra.Command + logger logging.Logger + outBuf bytes.Buffer + mockController *gomock.Controller + mockClient *testmocks.MockPackClient + cfg config.Config + imagePullPolicyHandler image.ImagePullPolicyHandler ) it.Before(func() { logger = logging.NewLogWithWriters(&outBuf, &outBuf) + imagePullPolicyHandler = fetcher_mock.NewMockPullPolicyManager(logger) cfg = config.Config{} mockController = gomock.NewController(t) mockClient = testmocks.NewMockPackClient(mockController) - command = commands.Rebase(logger, cfg, mockClient) + command = commands.Rebase(logger, cfg, mockClient, imagePullPolicyHandler) }) when("#RebaseCommand", func() { @@ -69,7 +72,7 @@ func testRebaseCommand(t *testing.T, when spec.G, it spec.S) { Image: runImage, Mirrors: []string{testMirror1, testMirror2}, }} - command = commands.Rebase(logger, cfg, mockClient) + command = commands.Rebase(logger, cfg, mockClient, imagePullPolicyHandler) repoName = "test/repo-image" opts = client.RebaseOptions{ @@ -109,7 +112,7 @@ func testRebaseCommand(t *testing.T, when spec.G, it spec.S) { Return(nil) cfg.PullPolicy = "if-not-present" - command = commands.Rebase(logger, cfg, mockClient) + command = commands.Rebase(logger, cfg, mockClient, imagePullPolicyHandler) command.SetArgs([]string{repoName, "--pull-policy", "never"}) h.AssertNil(t, command.Execute()) @@ -142,7 +145,7 @@ func testRebaseCommand(t *testing.T, when spec.G, it spec.S) { Return(nil) cfg.PullPolicy = "if-not-present" - command = commands.Rebase(logger, cfg, mockClient) + command = commands.Rebase(logger, cfg, mockClient, imagePullPolicyHandler) command.SetArgs([]string{repoName}) h.AssertNil(t, command.Execute()) @@ -152,7 +155,7 @@ func testRebaseCommand(t *testing.T, when spec.G, it spec.S) { it("passes it through", func() { opts.Force = true mockClient.EXPECT().Rebase(gomock.Any(), opts).Return(nil) - command = commands.Rebase(logger, cfg, mockClient) + command = commands.Rebase(logger, cfg, mockClient, imagePullPolicyHandler) command.SetArgs([]string{repoName, "--force"}) h.AssertNil(t, command.Execute()) }) @@ -170,7 +173,7 @@ func testRebaseCommand(t *testing.T, when spec.G, it spec.S) { Image: runImage, Mirrors: []string{testMirror1, testMirror2}, }} - command = commands.Rebase(logger, cfg, mockClient) + command = commands.Rebase(logger, cfg, mockClient, imagePullPolicyHandler) repoName = "test/repo-image" previousImage := "example.com/previous-image:tag" // Example of previous image with tag diff --git a/internal/commands/testdata/example_image.json b/internal/commands/testdata/example_image.json new file mode 100644 index 0000000000..383c664a7e --- /dev/null +++ b/internal/commands/testdata/example_image.json @@ -0,0 +1,13 @@ +{ + "interval": { + "pulling_interval": "1d", + "pruning_interval": "5d", + "last_prune": "2024-03-13T11:31:58+05:30" + }, + "image": { + "ImageIDtoTIME": { + "gcr.io/buildpacks/builder:v1": "2024-03-13T11:32:00+05:30", + "gcr.io/buildpacks/gcp/run:v1": "2024-03-13T11:32:03+05:30" + } + } +} \ No newline at end of file diff --git a/pkg/client/client.go b/pkg/client/client.go index ad974cd44b..4e7996ceed 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -94,13 +94,14 @@ type Client struct { logger logging.Logger docker DockerClient - keychain authn.Keychain - imageFactory ImageFactory - imageFetcher ImageFetcher - accessChecker AccessChecker - downloader BlobDownloader - lifecycleExecutor LifecycleExecutor - buildpackDownloader BuildpackDownloader + keychain authn.Keychain + imageFactory ImageFactory + imageFetcher ImageFetcher + imagePullPolicyHandler image.ImagePullPolicyHandler + accessChecker AccessChecker + downloader BlobDownloader + lifecycleExecutor LifecycleExecutor + buildpackDownloader BuildpackDownloader experimental bool registryMirrors map[string]string @@ -133,6 +134,14 @@ func WithFetcher(f ImageFetcher) Option { } } +// WithImagePullChecker supply your own ImagePullChecker. +// An ImagePullChecker provides functionality to check and manage image pulling intervals. +func WithImagePullChecker(i image.ImagePullPolicyHandler) Option { + return func(c *Client) { + c.imagePullPolicyHandler = i + } +} + // WithAccessChecker supply your own AccessChecker. // A AccessChecker returns true if an image is accessible for reading. func WithAccessChecker(f AccessChecker) Option { @@ -231,8 +240,7 @@ func NewClient(opts ...Option) (*Client, error) { } if client.imageFetcher == nil { - imagePullChecker := image.NewPullPolicyManager(client.logger) - client.imageFetcher = image.NewFetcher(client.logger, client.docker, imagePullChecker, image.WithRegistryMirrors(client.registryMirrors), image.WithKeychain(client.keychain)) + client.imageFetcher = image.NewFetcher(client.logger, client.docker, client.imagePullPolicyHandler, image.WithRegistryMirrors(client.registryMirrors), image.WithKeychain(client.keychain)) } if client.imageFactory == nil { diff --git a/pkg/image/fetcher.go b/pkg/image/fetcher.go index 9f26c2921e..2afad2444c 100644 --- a/pkg/image/fetcher.go +++ b/pkg/image/fetcher.go @@ -36,11 +36,13 @@ type LayoutOption struct { Sparse bool } -type ImagePullChecker interface { +type ImagePullPolicyHandler interface { + ParsePullPolicy(policy string) (PullPolicy, error) CheckImagePullInterval(imageID string, path string) (bool, error) - Read(path string) (*ImageJSON, error) - PruneOldImages(f *Fetcher) error + PruneOldImages(docker DockerClient) error UpdateImagePullRecord(path string, imageID string, timestamp string) error + UpdateImageJSONDuration(intervalStr string) error + Read(path string) (*ImageJSON, error) Write(imageJSON *ImageJSON, path string) error } @@ -48,7 +50,7 @@ func intervalPolicy(options FetchOptions) bool { return options.PullPolicy == PullWithInterval || options.PullPolicy == PullHourly || options.PullPolicy == PullDaily || options.PullPolicy == PullWeekly } -func NewPullPolicyManager(logger logging.Logger) *ImagePullPolicyManager { +func NewPullPolicyManager(logger logging.Logger) ImagePullPolicyHandler { return &ImagePullPolicyManager{Logger: logger} } @@ -75,7 +77,7 @@ type Fetcher struct { logger logging.Logger registryMirrors map[string]string keychain authn.Keychain - imagePullChecker ImagePullChecker + imagePullChecker ImagePullPolicyHandler } type FetchOptions struct { @@ -85,7 +87,7 @@ type FetchOptions struct { LayoutOption LayoutOption } -func NewFetcher(logger logging.Logger, docker DockerClient, imagePullChecker ImagePullChecker, opts ...FetcherOption) *Fetcher { +func NewFetcher(logger logging.Logger, docker DockerClient, imagePullChecker ImagePullPolicyHandler, opts ...FetcherOption) *Fetcher { fetcher := &Fetcher{ logger: logger, docker: docker, @@ -148,7 +150,7 @@ func (f *Fetcher) Fetch(ctx context.Context, name string, options FetchOptions) return img, err } - err = f.imagePullChecker.PruneOldImages(f) + err = f.imagePullChecker.PruneOldImages(f.docker) if err != nil { f.logger.Warnf("Failed to prune images, %s", err) } diff --git a/pkg/image/fetcher_test.go b/pkg/image/fetcher_test.go index f20ba35ace..bedcba7015 100644 --- a/pkg/image/fetcher_test.go +++ b/pkg/image/fetcher_test.go @@ -22,6 +22,7 @@ import ( "github.com/buildpacks/pack/pkg/image" "github.com/buildpacks/pack/pkg/logging" + "github.com/buildpacks/pack/pkg/testmocks" h "github.com/buildpacks/pack/testhelpers" ) @@ -29,74 +30,7 @@ var docker client.CommonAPIClient var logger logging.Logger var registryConfig *h.TestRegistryConfig var imageJSON *image.ImageJSON -var mockImagePullChecker = NewMockImagePullChecker(logger) - -type MockPullChecker struct { - *image.ImagePullPolicyManager - MockCheckImagePullInterval func(imageID string, path string) (bool, error) - MockRead func(path string) (*image.ImageJSON, error) - MockPruneOldImages func(f *image.Fetcher) error - MockUpdateImagePullRecord func(path string, imageID string, timestamp string) error - MockWrite func(imageJSON *image.ImageJSON, path string) error -} - -func NewMockImagePullChecker(logger logging.Logger) *MockPullChecker { - return &MockPullChecker{ - ImagePullPolicyManager: image.NewPullPolicyManager(logger), - } -} - -func (m *MockPullChecker) CheckImagePullInterval(imageID string, path string) (bool, error) { - if m.MockCheckImagePullInterval != nil { - return m.MockCheckImagePullInterval(imageID, path) - } - return false, nil -} - -func (m *MockPullChecker) Write(imageJSON *image.ImageJSON, path string) error { - if m.MockWrite != nil { - return m.MockWrite(imageJSON, path) - } - return nil -} - -func (m *MockPullChecker) Read(path string) (*image.ImageJSON, error) { - if m.MockRead != nil { - return m.MockRead(path) - } - - imageJSON = &image.ImageJSON{ - Interval: &image.Interval{ - PullingInterval: "7d", - PruningInterval: "7d", - LastPrune: "2023-01-01T00:00:00Z", - }, - Image: &image.ImageData{ - ImageIDtoTIME: map[string]string{ - "repoName": "2023-01-01T00:00:00Z", - }, - }, - } - - return imageJSON, nil -} - -func (m *MockPullChecker) PruneOldImages(f *image.Fetcher) error { - if m.MockPruneOldImages != nil { - return m.MockPruneOldImages(f) - } - - return nil -} - -func (m *MockPullChecker) UpdateImagePullRecord(path string, imageID string, timestamp string) error { - if m.MockUpdateImagePullRecord != nil { - fmt.Printf("checking wheather its calling or not") - return m.MockUpdateImagePullRecord(path, imageID, timestamp) - } - - return nil -} +var mockImagePullPolicyHandler = testmocks.NewMockPullPolicyManager(logger) func TestFetcher(t *testing.T) { color.Disable(true) @@ -129,7 +63,7 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { repo = "some-org/" + h.RandString(10) repoName = registryConfig.RepoName(repo) logger = logging.NewLogWithWriters(&outBuf, &outBuf) - imageFetcher = image.NewFetcher(logger, docker, mockImagePullChecker) + imageFetcher = image.NewFetcher(logger, docker, mockImagePullPolicyHandler) info, err := docker.Info(context.TODO()) h.AssertNil(t, err) @@ -240,10 +174,10 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { var outCons *color.Console outCons, output = h.MockWriterAndOutput() logger = logging.NewLogWithWriters(outCons, outCons) - mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, path string) (bool, error) { + mockImagePullPolicyHandler.MockCheckImagePullInterval = func(imageID string, path string) (bool, error) { return true, nil } - imageFetcher = image.NewFetcher(logger, docker, mockImagePullChecker) + imageFetcher = image.NewFetcher(logger, docker, mockImagePullPolicyHandler) }) it.After(func() { @@ -448,14 +382,14 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { when("there is no local image and CheckImagePullInterval returns true", func() { it.Before(func() { - mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, path string) (bool, error) { + mockImagePullPolicyHandler.MockCheckImagePullInterval = func(imageID string, path string) (bool, error) { return true, nil } - imageFetcher = image.NewFetcher(logging.NewLogWithWriters(&outBuf, &outBuf), docker, mockImagePullChecker) + imageFetcher = image.NewFetcher(logging.NewLogWithWriters(&outBuf, &outBuf), docker, mockImagePullPolicyHandler) }) it.After(func() { - mockImagePullChecker.MockCheckImagePullInterval = nil + mockImagePullPolicyHandler.MockCheckImagePullInterval = nil }) it("pulls the remote image and returns it", func() { @@ -470,7 +404,7 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { when("there is no local image and CheckImagePullInterval returns false", func() { it.Before(func() { - mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, path string) (bool, error) { + mockImagePullPolicyHandler.MockCheckImagePullInterval = func(imageID string, path string) (bool, error) { return false, nil } @@ -487,22 +421,22 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { }, } - imageFetcher = image.NewFetcher(logging.NewLogWithWriters(&outBuf, &outBuf), docker, mockImagePullChecker) + imageFetcher = image.NewFetcher(logging.NewLogWithWriters(&outBuf, &outBuf), docker, mockImagePullPolicyHandler) - mockImagePullChecker.MockRead = func(path string) (*image.ImageJSON, error) { + mockImagePullPolicyHandler.MockRead = func(path string) (*image.ImageJSON, error) { return imageJSON, nil } }) it.After(func() { - mockImagePullChecker.MockCheckImagePullInterval = nil - mockImagePullChecker.MockRead = nil + mockImagePullPolicyHandler.MockCheckImagePullInterval = nil + mockImagePullPolicyHandler.MockRead = nil }) it("returns an error and deletes the image record", func() { _, err := imageFetcher.Fetch(context.TODO(), repoName, image.FetchOptions{Daemon: true, PullPolicy: image.PullWeekly}) h.AssertError(t, err, fmt.Sprintf("image '%s' does not exist on the daemon", repoName)) - imageJSON, err = mockImagePullChecker.Read("") + imageJSON, err = mockImagePullPolicyHandler.Read("") h.AssertNil(t, err) _, exists := imageJSON.Image.ImageIDtoTIME[repoName] h.AssertEq(t, exists, false) @@ -515,7 +449,7 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, err) h.AssertNil(t, img.Save()) - mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, path string) (bool, error) { + mockImagePullPolicyHandler.MockCheckImagePullInterval = func(imageID string, path string) (bool, error) { return true, nil } @@ -532,23 +466,23 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { }, } - mockImagePullChecker.MockRead = func(path string) (*image.ImageJSON, error) { + mockImagePullPolicyHandler.MockRead = func(path string) (*image.ImageJSON, error) { return imageJSON, nil } - mockImagePullChecker.MockUpdateImagePullRecord = func(path string, imageID string, timestamp string) error { - imageJSON, _ = mockImagePullChecker.Read("") + mockImagePullPolicyHandler.MockUpdateImagePullRecord = func(path string, imageID string, timestamp string) error { + imageJSON, _ = mockImagePullPolicyHandler.Read("") imageJSON.Image.ImageIDtoTIME[repoName] = timestamp return nil } - imageFetcher = image.NewFetcher(logging.NewLogWithWriters(&outBuf, &outBuf), docker, mockImagePullChecker) + imageFetcher = image.NewFetcher(logging.NewLogWithWriters(&outBuf, &outBuf), docker, mockImagePullPolicyHandler) }) it.After(func() { - mockImagePullChecker.MockCheckImagePullInterval = nil - mockImagePullChecker.MockRead = nil - mockImagePullChecker.MockUpdateImagePullRecord = nil + mockImagePullPolicyHandler.MockCheckImagePullInterval = nil + mockImagePullPolicyHandler.MockRead = nil + mockImagePullPolicyHandler.MockUpdateImagePullRecord = nil h.DockerRmi(docker, repoName) }) @@ -558,7 +492,7 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { _, err := imageFetcher.Fetch(context.TODO(), repoName, image.FetchOptions{Daemon: true, PullPolicy: image.PullWeekly}) h.AssertNil(t, err) - imageJSON, _ = mockImagePullChecker.Read("") + imageJSON, _ = mockImagePullPolicyHandler.Read("") afterFetch, _ := time.Parse(time.RFC3339, imageJSON.Image.ImageIDtoTIME[repoName]) fmt.Printf("after fetch: %v\n", imageJSON.Image.ImageIDtoTIME[repoName]) @@ -574,14 +508,14 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, localImg.SetLabel(label, "2")) h.AssertNil(t, localImg.Save()) - mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, path string) (bool, error) { + mockImagePullPolicyHandler.MockCheckImagePullInterval = func(imageID string, path string) (bool, error) { return true, nil } - imageFetcher = image.NewFetcher(logging.NewLogWithWriters(&outBuf, &outBuf), docker, mockImagePullChecker) + imageFetcher = image.NewFetcher(logging.NewLogWithWriters(&outBuf, &outBuf), docker, mockImagePullPolicyHandler) }) it.After(func() { - mockImagePullChecker.MockCheckImagePullInterval = nil + mockImagePullPolicyHandler.MockCheckImagePullInterval = nil h.DockerRmi(docker, repoName) }) @@ -601,14 +535,14 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { when("there is no local image and CheckImagePullInterval returns true", func() { it.Before(func() { - mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, path string) (bool, error) { + mockImagePullPolicyHandler.MockCheckImagePullInterval = func(imageID string, path string) (bool, error) { return true, nil } - imageFetcher = image.NewFetcher(logging.NewLogWithWriters(&outBuf, &outBuf), docker, mockImagePullChecker) + imageFetcher = image.NewFetcher(logging.NewLogWithWriters(&outBuf, &outBuf), docker, mockImagePullPolicyHandler) }) it.After(func() { - mockImagePullChecker.MockCheckImagePullInterval = nil + mockImagePullPolicyHandler.MockCheckImagePullInterval = nil }) it("try to pull the remote image and returns error", func() { @@ -619,7 +553,7 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { when("there is no local image and CheckImagePullInterval returns false", func() { it.Before(func() { - mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, path string) (bool, error) { + mockImagePullPolicyHandler.MockCheckImagePullInterval = func(imageID string, path string) (bool, error) { return false, nil } @@ -636,21 +570,21 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { }, } - imageFetcher = image.NewFetcher(logging.NewLogWithWriters(&outBuf, &outBuf), docker, mockImagePullChecker) - mockImagePullChecker.MockRead = func(path string) (*image.ImageJSON, error) { + imageFetcher = image.NewFetcher(logging.NewLogWithWriters(&outBuf, &outBuf), docker, mockImagePullPolicyHandler) + mockImagePullPolicyHandler.MockRead = func(path string) (*image.ImageJSON, error) { return imageJSON, nil } }) it.After(func() { - mockImagePullChecker.MockCheckImagePullInterval = nil - mockImagePullChecker.MockRead = nil + mockImagePullPolicyHandler.MockCheckImagePullInterval = nil + mockImagePullPolicyHandler.MockRead = nil }) it("returns an error and deletes the image record", func() { _, err := imageFetcher.Fetch(context.TODO(), repoName, image.FetchOptions{Daemon: true, PullPolicy: image.PullWeekly}) h.AssertError(t, err, fmt.Sprintf("image '%s' does not exist on the daemon", repoName)) - imageJSON, err = mockImagePullChecker.Read("") + imageJSON, err = mockImagePullPolicyHandler.Read("") h.AssertNil(t, err) _, exists := imageJSON.Image.ImageIDtoTIME[repoName] h.AssertEq(t, exists, false) @@ -664,14 +598,14 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, localImg.SetLabel(label, "2")) h.AssertNil(t, localImg.Save()) - mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, path string) (bool, error) { + mockImagePullPolicyHandler.MockCheckImagePullInterval = func(imageID string, path string) (bool, error) { return true, nil } - imageFetcher = image.NewFetcher(logging.NewLogWithWriters(&outBuf, &outBuf), docker, mockImagePullChecker) + imageFetcher = image.NewFetcher(logging.NewLogWithWriters(&outBuf, &outBuf), docker, mockImagePullPolicyHandler) }) it.After(func() { - mockImagePullChecker.MockCheckImagePullInterval = nil + mockImagePullPolicyHandler.MockCheckImagePullInterval = nil h.DockerRmi(docker, repoName) }) @@ -692,14 +626,14 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, localImg.SetLabel(label, "2")) h.AssertNil(t, localImg.Save()) - mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, path string) (bool, error) { + mockImagePullPolicyHandler.MockCheckImagePullInterval = func(imageID string, path string) (bool, error) { return true, nil } - imageFetcher = image.NewFetcher(logging.NewLogWithWriters(&outBuf, &outBuf), docker, mockImagePullChecker) + imageFetcher = image.NewFetcher(logging.NewLogWithWriters(&outBuf, &outBuf), docker, mockImagePullPolicyHandler) }) it.After(func() { - mockImagePullChecker.MockCheckImagePullInterval = nil + mockImagePullPolicyHandler.MockCheckImagePullInterval = nil h.DockerRmi(docker, repoName) }) diff --git a/pkg/image/pull_policy.go b/pkg/image/pull_policy.go index bca560b85b..1d9a47bff4 100644 --- a/pkg/image/pull_policy.go +++ b/pkg/image/pull_policy.go @@ -12,6 +12,8 @@ import ( "github.com/pkg/errors" + "github.com/buildpacks/imgutil/local" + "github.com/buildpacks/pack/internal/config" "github.com/buildpacks/pack/pkg/logging" ) @@ -75,24 +77,22 @@ func DefaultImageJSONPath() (string, error) { var nameMap = map[string]PullPolicy{"always": PullAlways, "hourly": PullHourly, "daily": PullDaily, "weekly": PullWeekly, "never": PullNever, "if-not-present": PullIfNotPresent, "": PullAlways} // ParsePullPolicy from string with support for interval formats -func ParsePullPolicy(policy string, logger logging.Logger) (PullPolicy, error) { - pullPolicyManager := NewPullPolicyManager(logger) - +func (i *ImagePullPolicyManager) ParsePullPolicy(policy string) (PullPolicy, error) { if val, ok := nameMap[policy]; ok { if val == PullHourly { - err := pullPolicyManager.updateImageJSONDuration(hourly) + err := i.UpdateImageJSONDuration(hourly) if err != nil { return PullAlways, err } } if val == PullDaily { - err := pullPolicyManager.updateImageJSONDuration(daily) + err := i.UpdateImageJSONDuration(daily) if err != nil { return PullAlways, err } } if val == PullWeekly { - err := pullPolicyManager.updateImageJSONDuration(weekly) + err := i.UpdateImageJSONDuration(weekly) if err != nil { return PullAlways, err } @@ -109,7 +109,7 @@ func ParsePullPolicy(policy string, logger logging.Logger) (PullPolicy, error) { return PullAlways, errors.Errorf("invalid interval format: %s", intervalStr) } - err := pullPolicyManager.updateImageJSONDuration(intervalStr) + err := i.UpdateImageJSONDuration(intervalStr) if err != nil { return PullAlways, err } @@ -141,7 +141,7 @@ func (p PullPolicy) String() string { return "" } -func (i *ImagePullPolicyManager) updateImageJSONDuration(intervalStr string) error { +func (i *ImagePullPolicyManager) UpdateImageJSONDuration(intervalStr string) error { path, err := DefaultImageJSONPath() if err != nil { return err @@ -188,7 +188,7 @@ func parseDurationString(durationStr string) (time.Duration, error) { return time.Duration(totalMinutes) * time.Minute, nil } -func (i *ImagePullPolicyManager) PruneOldImages(f *Fetcher) error { +func (i *ImagePullPolicyManager) PruneOldImages(docker DockerClient) error { path, err := DefaultImageJSONPath() if err != nil { return err @@ -229,8 +229,11 @@ func (i *ImagePullPolicyManager) PruneOldImages(f *Fetcher) error { return errors.Wrap(err, "failed to parse image timestamp fron JSON") } - _, err = f.fetchDaemonImage(imageID) - if !errors.Is(err, ErrNotFound) { + image, err := local.NewImage(imageID, docker, local.FromBaseImage(imageID)) + if err != nil { + return err + } + if !image.Found() { delete(imageJSON.Image.ImageIDtoTIME, imageID) } diff --git a/pkg/image/pull_policy_test.go b/pkg/image/pull_policy_test.go index 41fde7fddf..6fe10fca6f 100644 --- a/pkg/image/pull_policy_test.go +++ b/pkg/image/pull_policy_test.go @@ -1,12 +1,15 @@ package image_test import ( + "bytes" "testing" "github.com/sclevine/spec" "github.com/sclevine/spec/report" "github.com/buildpacks/pack/pkg/image" + "github.com/buildpacks/pack/pkg/logging" + fetcher_mock "github.com/buildpacks/pack/pkg/testmocks" h "github.com/buildpacks/pack/testhelpers" ) @@ -16,55 +19,66 @@ func TestPullPolicy(t *testing.T) { func testPullPolicy(t *testing.T, when spec.G, it spec.S) { when("#ParsePullPolicy", func() { + var ( + outBuf bytes.Buffer + logger logging.Logger + imagePullChecker image.ImagePullPolicyHandler + ) + + it.Before(func() { + logger = logging.NewLogWithWriters(&outBuf, &outBuf) + imagePullChecker = fetcher_mock.NewMockPullPolicyManager(logger) + }) + it("returns PullNever for never", func() { - policy, err := image.ParsePullPolicy("never", logger) + policy, err := imagePullChecker.ParsePullPolicy("never") h.AssertNil(t, err) h.AssertEq(t, policy, image.PullNever) }) it("returns PullAlways for always", func() { - policy, err := image.ParsePullPolicy("always", logger) + policy, err := imagePullChecker.ParsePullPolicy("always") h.AssertNil(t, err) h.AssertEq(t, policy, image.PullAlways) }) it("returns PullIfNotPresent for if-not-present", func() { - policy, err := image.ParsePullPolicy("if-not-present", logger) + policy, err := imagePullChecker.ParsePullPolicy("if-not-present") h.AssertNil(t, err) h.AssertEq(t, policy, image.PullIfNotPresent) }) it("returns PullHourly for hourly", func() { - policy, err := image.ParsePullPolicy("hourly", logger) + policy, err := imagePullChecker.ParsePullPolicy("hourly") h.AssertNil(t, err) h.AssertEq(t, policy, image.PullHourly) }) it("returns PullDaily for daily", func() { - policy, err := image.ParsePullPolicy("daily", logger) + policy, err := imagePullChecker.ParsePullPolicy("daily") h.AssertNil(t, err) h.AssertEq(t, policy, image.PullDaily) }) it("returns PullWeekly for weekly", func() { - policy, err := image.ParsePullPolicy("weekly", logger) + policy, err := imagePullChecker.ParsePullPolicy("weekly") h.AssertNil(t, err) h.AssertEq(t, policy, image.PullWeekly) }) it("returns PullWithInterval for interval= format", func() { - policy, err := image.ParsePullPolicy("interval=4d", logger) + policy, err := imagePullChecker.ParsePullPolicy("interval=4d") h.AssertNil(t, err) h.AssertEq(t, policy, image.PullWithInterval) }) it("returns error for unknown string", func() { - _, err := image.ParsePullPolicy("fake-policy-here", logger) + _, err := imagePullChecker.ParsePullPolicy("fake-policy-here") h.AssertError(t, err, "invalid pull policy") }) it("returns error for invalid interval format", func() { - _, err := image.ParsePullPolicy("interval=invalid", logger) + _, err := imagePullChecker.ParsePullPolicy("interval=invalid") h.AssertError(t, err, "invalid interval format") }) }) diff --git a/pkg/testmocks/mock_image_fetcher.go b/pkg/testmocks/mock_image_fetcher.go index 281f28d04d..90225529e2 100644 --- a/pkg/testmocks/mock_image_fetcher.go +++ b/pkg/testmocks/mock_image_fetcher.go @@ -11,9 +11,13 @@ import ( imgutil "github.com/buildpacks/imgutil" gomock "github.com/golang/mock/gomock" + "github.com/buildpacks/pack/pkg/logging" + image "github.com/buildpacks/pack/pkg/image" ) +var imageJSON *image.ImageJSON + // MockImageFetcher is a mock of ImageFetcher interface. type MockImageFetcher struct { ctrl *gomock.Controller @@ -25,6 +29,17 @@ type MockImageFetcherMockRecorder struct { mock *MockImageFetcher } +type MockImagePullPolicyHandler struct { + *image.ImagePullPolicyManager + MockParsePullPolicy func(policy string, logger logging.Logger) (image.PullPolicy, error) + MockCheckImagePullInterval func(imageID string, path string) (bool, error) + MockPruneOldImages func(docker *image.DockerClient) error + MockUpdateImagePullRecord func(path string, imageID string, timestamp string) error + MockUpdateImageJSONDuration func(intervalStr string) error + MockRead func(path string) (*image.ImageJSON, error) + MockWrite func(imageJSON *image.ImageJSON, path string) error +} + // NewMockImageFetcher creates a new mock instance. func NewMockImageFetcher(ctrl *gomock.Controller) *MockImageFetcher { mock := &MockImageFetcher{ctrl: ctrl} @@ -51,3 +66,66 @@ func (mr *MockImageFetcherMockRecorder) Fetch(arg0, arg1, arg2 interface{}) *gom mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Fetch", reflect.TypeOf((*MockImageFetcher)(nil).Fetch), arg0, arg1, arg2) } + +func NewMockPullPolicyManager(logger logging.Logger) *MockImagePullPolicyHandler { + return &MockImagePullPolicyHandler{} +} + +func (m *MockImagePullPolicyHandler) CheckImagePullInterval(imageID string, path string) (bool, error) { + if m.MockCheckImagePullInterval != nil { + return m.MockCheckImagePullInterval(imageID, path) + } + return false, nil +} + +func (m *MockImagePullPolicyHandler) Write(imageJSON *image.ImageJSON, path string) error { + if m.MockWrite != nil { + return m.MockWrite(imageJSON, path) + } + return nil +} + +func (m *MockImagePullPolicyHandler) Read(path string) (*image.ImageJSON, error) { + if m.MockRead != nil { + return m.MockRead(path) + } + + imageJSON = &image.ImageJSON{ + Interval: &image.Interval{ + PullingInterval: "7d", + PruningInterval: "7d", + LastPrune: "2023-01-01T00:00:00Z", + }, + Image: &image.ImageData{ + ImageIDtoTIME: map[string]string{ + "repoName": "2023-01-01T00:00:00Z", + }, + }, + } + + return imageJSON, nil +} + +func (m *MockImagePullPolicyHandler) PruneOldImages(docker image.DockerClient) error { + if m.MockPruneOldImages != nil { + return m.MockPruneOldImages(&docker) + } + + return m.ImagePullPolicyManager.PruneOldImages(docker) +} + +func (m *MockImagePullPolicyHandler) UpdateImagePullRecord(path string, imageID string, timestamp string) error { + if m.MockUpdateImagePullRecord != nil { + return m.MockUpdateImagePullRecord(path, imageID, timestamp) + } + + return nil +} + +func (m *MockImagePullPolicyHandler) UpdateImageJSONDuration(intervalStr string) error { + if m.MockUpdateImageJSONDuration != nil { + return m.MockUpdateImageJSONDuration(intervalStr) + } + + return nil +} From 8ba2e926a0a2381fb27f8119d549029d10ef9a0a Mon Sep 17 00:00:00 2001 From: Parthiba-Hazra Date: Tue, 23 Apr 2024 20:51:49 +0530 Subject: [PATCH 11/11] Refactoring newly added pull policy - removing interval pull policy as we can cover 95% of use cases using the hourly, daily, weekly pull policies and also removing the pruning interval command as it will set to be constant 7 days. Signed-off-by: Parthiba-Hazra --- cmd/cmd.go | 2 +- go.mod | 18 +- go.sum | 20 +-- internal/commands/config_prune_interval.go | 102 ----------- .../commands/config_prune_interval_test.go | 170 ------------------ pkg/image/fetcher.go | 19 +- pkg/image/fetcher_test.go | 5 +- pkg/image/pull_policy.go | 132 ++------------ pkg/image/pull_policy_test.go | 2 +- pkg/testmocks/mock_image_fetcher.go | 83 +-------- .../mock_image_pull_policy_handler.go | 137 ++++++++++++++ 11 files changed, 190 insertions(+), 500 deletions(-) delete mode 100644 internal/commands/config_prune_interval.go delete mode 100644 internal/commands/config_prune_interval_test.go create mode 100644 pkg/testmocks/mock_image_pull_policy_handler.go diff --git a/cmd/cmd.go b/cmd/cmd.go index 7108b81afd..adcb5de82d 100644 --- a/cmd/cmd.go +++ b/cmd/cmd.go @@ -139,7 +139,7 @@ func initConfig() (config.Config, string, error) { return cfg, path, nil } -func initClient(logger logging.Logger, cfg config.Config) (*client.Client, error) { +func initClient(logger logging.Logger, cfg config.Config, imagePullPolicyHandler image.ImagePullPolicyHandler) (*client.Client, error) { if err := client.ProcessDockerContext(logger); err != nil { return nil, err } diff --git a/go.mod b/go.mod index 019c2e0264..f860f5c851 100644 --- a/go.mod +++ b/go.mod @@ -11,7 +11,7 @@ require ( github.com/docker/docker v25.0.5+incompatible github.com/docker/go-connections v0.5.0 github.com/dustin/go-humanize v1.0.1 - github.com/gdamore/tcell/v2 v2.7.4 + github.com/gdamore/tcell/v2 v2.7.1 github.com/go-git/go-git/v5 v5.11.0 github.com/golang/mock v1.6.0 github.com/google/go-cmp v0.6.0 @@ -29,9 +29,9 @@ require ( github.com/sabhiram/go-gitignore v0.0.0-20210923224102-525f6e181f06 github.com/sclevine/spec v1.4.0 github.com/spf13/cobra v1.8.0 - golang.org/x/crypto v0.21.0 - golang.org/x/mod v0.16.0 - golang.org/x/oauth2 v0.18.0 + golang.org/x/crypto v0.20.0 + golang.org/x/mod v0.15.0 + golang.org/x/oauth2 v0.17.0 golang.org/x/sync v0.6.0 golang.org/x/sys v0.18.0 golang.org/x/term v0.18.0 @@ -128,11 +128,11 @@ require ( github.com/spf13/pflag v1.0.5 // indirect github.com/vbatts/tar-split v0.11.5 // indirect github.com/xanzy/ssh-agent v0.3.3 // indirect - go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.47.0 // indirect - go.opentelemetry.io/otel v1.22.0 // indirect - go.opentelemetry.io/otel/metric v1.22.0 // indirect - go.opentelemetry.io/otel/trace v1.22.0 // indirect - golang.org/x/net v0.22.0 // indirect + go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.48.0 // indirect + go.opentelemetry.io/otel v1.23.0 // indirect + go.opentelemetry.io/otel/metric v1.23.0 // indirect + go.opentelemetry.io/otel/trace v1.23.0 // indirect + golang.org/x/net v0.21.0 // indirect golang.org/x/tools v0.18.0 // indirect google.golang.org/appengine v1.6.8 // indirect google.golang.org/protobuf v1.33.0 // indirect diff --git a/go.sum b/go.sum index b78983320a..17ca08fc80 100644 --- a/go.sum +++ b/go.sum @@ -154,8 +154,8 @@ github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMo github.com/gdamore/encoding v1.0.0 h1:+7OoQ1Bc6eTm5niUzBa0Ctsh6JbMW6Ra+YNuAtDBdko= github.com/gdamore/encoding v1.0.0/go.mod h1:alR0ol34c49FCSBLjhosxzcPHQbf2trDkoo5dl+VrEg= github.com/gdamore/tcell/v2 v2.4.1-0.20210905002822-f057f0a857a1/go.mod h1:Az6Jt+M5idSED2YPGtwnfJV0kXohgdCBPmHGSYc1r04= -github.com/gdamore/tcell/v2 v2.7.4 h1:sg6/UnTM9jGpZU+oFYAsDahfchWAFW8Xx2yFinNSAYU= -github.com/gdamore/tcell/v2 v2.7.4/go.mod h1:dSXtXTSK0VsW1biw65DZLZ2NKr7j0qP/0J7ONmsraWg= +github.com/gdamore/tcell/v2 v2.7.1 h1:TiCcmpWHiAU7F0rA2I3S2Y4mmLmO9KHxJ7E1QhYzQbc= +github.com/gdamore/tcell/v2 v2.7.1/go.mod h1:dSXtXTSK0VsW1biw65DZLZ2NKr7j0qP/0J7ONmsraWg= github.com/gliderlabs/ssh v0.3.5 h1:OcaySEmAQJgyYcArR+gGGTHCyE7nvhEMTlYY+Dp8CpY= github.com/gliderlabs/ssh v0.3.5/go.mod h1:8XB4KraRrX39qHhT6yxPsHedjA08I/uBVwj4xC+/+z4= github.com/go-git/gcfg v1.5.1-0.20230307220236-3a3c6141e376 h1:+zs/tPmkDkHx3U66DAb0lQFJrpS6731Oaa12ikc+DiI= @@ -417,15 +417,15 @@ golang.org/x/crypto v0.0.0-20220722155217-630584e8d5aa/go.mod h1:IxCIyHEi3zRg3s0 golang.org/x/crypto v0.3.1-0.20221117191849-2c476679df9a/go.mod h1:hebNnKkNXi2UzZN1eVRvBB7co0a+JxK6XbPiWVs/3J4= golang.org/x/crypto v0.6.0/go.mod h1:OFC/31mSvZgRz0V1QTNCzfAI1aIRzbiufJtkMIlEp58= golang.org/x/crypto v0.7.0/go.mod h1:pYwdfH91IfpZVANVyUOhSIPZaFoJGxTFbZhFTx+dXZU= -golang.org/x/crypto v0.21.0 h1:X31++rzVUdKhX5sWmSOFZxx8UW/ldWx55cbf08iNAMA= -golang.org/x/crypto v0.21.0/go.mod h1:0BP7YvVV9gBbVKyeTG0Gyn+gZm94bibOW5BjDEYAOMs= +golang.org/x/crypto v0.20.0 h1:jmAMJJZXr5KiCw05dfYK9QnqaqKLYXijU23lsEdcQqg= +golang.org/x/crypto v0.20.0/go.mod h1:Xwo95rrVNIoSMx9wa1JroENMToLWn3RNVrTBpLHgZPQ= golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= -golang.org/x/mod v0.16.0 h1:QX4fJ0Rr5cPQCF7O9lh9Se4pmwfwskqZfq5moyldzic= -golang.org/x/mod v0.16.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c= +golang.org/x/mod v0.15.0 h1:SernR4v+D55NyBH2QiEQrlBAnj1ECL6AGrA5+dPaMY8= +golang.org/x/mod v0.15.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c= golang.org/x/net v0.0.0-20180906233101-161cd47e91fd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20181114220301-adae6a3d119a/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= @@ -441,11 +441,11 @@ golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug golang.org/x/net v0.2.0/go.mod h1:KqCZLdyyvdV855qA2rE3GC2aiw5xGR5TEjj8smXukLY= golang.org/x/net v0.6.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs= golang.org/x/net v0.8.0/go.mod h1:QVkue5JL9kW//ek3r6jTKnTFis1tRmNAW2P1shuFdJc= -golang.org/x/net v0.22.0 h1:9sGLhx7iRIHEiX0oAJ3MRZMUCElJgy7Br1nO+AMN3Tc= -golang.org/x/net v0.22.0/go.mod h1:JKghWKKOSdJwpW2GEx0Ja7fmaKnMsbu+MWVZTokSYmg= +golang.org/x/net v0.21.0 h1:AQyQV4dYCvJ7vGmJyKki9+PBdyvhkSd8EIx/qb0AYv4= +golang.org/x/net v0.21.0/go.mod h1:bIjVDfnllIU7BJ2DNgfnXvpSvtn8VRwhlsaeUTyUS44= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= -golang.org/x/oauth2 v0.18.0 h1:09qnuIAgzdx1XplqJvW6CQqMCtGZykZWcXzPMPUusvI= -golang.org/x/oauth2 v0.18.0/go.mod h1:Wf7knwG0MPoWIMMBgFlEaSUDaKskp0dCfrlJRJXbBi8= +golang.org/x/oauth2 v0.17.0 h1:6m3ZPmLEFdVxKKWnKq4VqZ60gutO35zm+zrAHVmHyDQ= +golang.org/x/oauth2 v0.17.0/go.mod h1:OzPDGQiuQMguemayvdylqddI7qcD9lnSDb+1FiwQ5HA= golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= diff --git a/internal/commands/config_prune_interval.go b/internal/commands/config_prune_interval.go deleted file mode 100644 index 5e00527e59..0000000000 --- a/internal/commands/config_prune_interval.go +++ /dev/null @@ -1,102 +0,0 @@ -package commands - -import ( - "fmt" - "regexp" - - "github.com/pkg/errors" - "github.com/spf13/cobra" - - "github.com/buildpacks/pack/internal/config" - "github.com/buildpacks/pack/internal/style" - "github.com/buildpacks/pack/pkg/image" - "github.com/buildpacks/pack/pkg/logging" -) - -func ConfigPruneInterval(logger logging.Logger, cfg config.Config, cfgPath string, imagePullPolicyHandler image.ImagePullPolicyHandler) *cobra.Command { - var unset bool - var intervalRegex = regexp.MustCompile(`^(\d+d)?(\d+h)?(\d+m)?$`) - - cmd := &cobra.Command{ - Use: "prune-interval", - Args: cobra.MaximumNArgs(1), - Short: "List, set, and unset the global pruning interval used for cleaning up outdated image entries from $HOME/.pack/image.json file", - Long: "You can use this command to list, set, and unset the default pruning interval for cleaning up unused images:\n" + - "* To list your current pruning interval, run `pack config prune-interval`.\n" + - "* To set a new pruning interval, run `pack config prune-interval ` where is a duration string (e.g., '7d' for 7 days).\n" + - "* To unset the pruning interval, run `pack config prune-interval --unset`.\n" + - fmt.Sprintf("Unsetting the pruning interval will reset the interval to the default, which is %s.", style.Symbol("7 days")), - RunE: logError(logger, func(cmd *cobra.Command, args []string) error { - imageJSONPath, err := image.DefaultImageJSONPath() - if err != nil { - return err - } - - switch { - case unset: - if len(args) > 0 { - return errors.Errorf("prune interval and --unset cannot be specified simultaneously") - } - imageJSON, err := imagePullPolicyHandler.Read(imageJSONPath) - if err != nil { - return err - } - oldPruneInterval := imageJSON.Interval.PruningInterval - imageJSON.Interval.PruningInterval = "7d" - - err = imagePullPolicyHandler.Write(imageJSON, imageJSONPath) - if err != nil { - return err - } - logger.Infof("Successfully unset pruning interval %s", style.Symbol(oldPruneInterval)) - logger.Infof("Pruning interval has been set to %s", style.Symbol(imageJSON.Interval.PruningInterval)) - case len(args) == 0: // list - imageJSON, err := imagePullPolicyHandler.Read(imageJSONPath) - if err != nil { - return err - } - pruneInterval := imageJSON.Interval.PruningInterval - if err != nil { - return err - } - - logger.Infof("The current prune interval is %s", style.Symbol(pruneInterval)) - default: // set - newPruneInterval := args[0] - - imageJSON, err := imagePullPolicyHandler.Read(imageJSONPath) - if err != nil { - return err - } - pruneInterval := imageJSON.Interval.PruningInterval - if err != nil { - return err - } - - if newPruneInterval == pruneInterval { - logger.Infof("Prune Interval is already set to %s", style.Symbol(newPruneInterval)) - return nil - } - - matches := intervalRegex.FindStringSubmatch(newPruneInterval) - if len(matches) == 0 { - return errors.Errorf("invalid interval format: %s", newPruneInterval) - } - - imageJSON.Interval.PruningInterval = newPruneInterval - - err = imagePullPolicyHandler.Write(imageJSON, imageJSONPath) - if err != nil { - return err - } - - logger.Infof("Successfully set %s as the pruning interval", style.Symbol(imageJSON.Interval.PruningInterval)) - } - - return nil - }), - } - cmd.Flags().BoolVarP(&unset, "unset", "u", false, "Unset prune interval, and set it back to the default prune-interval, which is "+style.Symbol("7d")) - AddHelpFlag(cmd, "prune-interval") - return cmd -} diff --git a/internal/commands/config_prune_interval_test.go b/internal/commands/config_prune_interval_test.go deleted file mode 100644 index 0f0d7a4b82..0000000000 --- a/internal/commands/config_prune_interval_test.go +++ /dev/null @@ -1,170 +0,0 @@ -package commands_test - -import ( - "bytes" - "os" - "path/filepath" - "testing" - - "github.com/sclevine/spec" - "github.com/sclevine/spec/report" - "github.com/spf13/cobra" - - "github.com/buildpacks/pack/internal/commands" - "github.com/buildpacks/pack/internal/config" - "github.com/buildpacks/pack/pkg/image" - "github.com/buildpacks/pack/pkg/logging" - fetcher_mock "github.com/buildpacks/pack/pkg/testmocks" - h "github.com/buildpacks/pack/testhelpers" -) - -var imageJSON *image.ImageJSON - -func TestConfigPruneInterval(t *testing.T) { - spec.Run(t, "ConfigPruneIntervalCommand", testConfigPruneIntervalCommand, spec.Random(), spec.Report(report.Terminal{})) -} - -func testConfigPruneIntervalCommand(t *testing.T, when spec.G, it spec.S) { - var ( - command *cobra.Command - logger logging.Logger - outBuf bytes.Buffer - tempPackHome string - configFile string - assert = h.NewAssertionManager(t) - cfg = config.Config{} - imagePullPolicyHandler *fetcher_mock.MockImagePullPolicyHandler - ) - - it.Before(func() { - logger = logging.NewLogWithWriters(&outBuf, &outBuf) - imagePullPolicyHandler = fetcher_mock.NewMockPullPolicyManager(logger) - tempPackHome, _ = os.MkdirTemp("", "pack-home") - configFile = filepath.Join(tempPackHome, "config.toml") - - command = commands.ConfigPruneInterval(logger, cfg, configFile, imagePullPolicyHandler) - }) - - it.After(func() { - _ = os.RemoveAll(tempPackHome) - }) - - when("#ConfigPruneInterval", func() { - when("no arguments are provided", func() { - it.Before(func() { - interval := "5d" - command.SetArgs([]string{interval}) - - err := command.Execute() - assert.Nil(err) - }) - - it.After(func() { - command.SetArgs([]string{"--unset"}) - - err := command.Execute() - assert.Nil(err) - }) - - it("lists the current pruning interval", func() { - command.SetArgs([]string{}) - - err := command.Execute() - - assert.Nil(err) - assert.Contains(outBuf.String(), "The current prune interval is") - }) - }) - - when("an argument is provided", func() { - when("argument is valid", func() { - it.After(func() { - command.SetArgs([]string{"--unset"}) - - err := command.Execute() - assert.Nil(err) - }) - - it("sets the provided interval as the pruning interval", func() { - interval := "5d" - command.SetArgs([]string{interval}) - - err := command.Execute() - - assert.Nil(err) - assert.Contains(outBuf.String(), "Successfully set") - }) - }) - - when("argument is invalid", func() { - it("returns an error", func() { - interval := "invalid" - command.SetArgs([]string{interval}) - - err := command.Execute() - - assert.Error(err) - assert.Contains(err.Error(), "invalid interval format") - }) - }) - - when("argument is valid and the same as the already set pruning interval", func() { - it.Before(func() { - imageJSON = &image.ImageJSON{ - Interval: &image.Interval{ - PullingInterval: "7d", - PruningInterval: "5d", - LastPrune: "2023-01-01T00:00:00Z", - }, - Image: &image.ImageData{ - ImageIDtoTIME: map[string]string{}, - }, - } - - imagePullPolicyHandler.MockRead = func(path string) (*image.ImageJSON, error) { - return imageJSON, nil - } - }) - - it.After(func() { - command.SetArgs([]string{"--unset"}) - - err := command.Execute() - assert.Nil(err) - imagePullPolicyHandler.MockRead = nil - }) - - it("sets the provided interval as the pruning interval", func() { - interval := "5d" - command.SetArgs([]string{interval}) - - err := command.Execute() - assert.Nil(err) - assert.Contains(outBuf.String(), "Prune Interval is already set to") - }) - }) - }) - - when("--unset flag is provided", func() { - it("unsets the pruning interval", func() { - command.SetArgs([]string{"--unset"}) - - err := command.Execute() - - assert.Nil(err) - assert.Contains(outBuf.String(), "Successfully unset pruning interval") - }) - }) - - when("both interval and --unset flag are provided", func() { - it("returns an error", func() { - command.SetArgs([]string{"5d", "--unset"}) - - err := command.Execute() - - assert.Error(err) - assert.Contains(err.Error(), "prune interval and --unset cannot be specified simultaneously") - }) - }) - }) -} diff --git a/pkg/image/fetcher.go b/pkg/image/fetcher.go index 2afad2444c..339824e261 100644 --- a/pkg/image/fetcher.go +++ b/pkg/image/fetcher.go @@ -38,16 +38,16 @@ type LayoutOption struct { type ImagePullPolicyHandler interface { ParsePullPolicy(policy string) (PullPolicy, error) - CheckImagePullInterval(imageID string, path string) (bool, error) + CheckImagePullInterval(imageID string, path string, pullPolicy PullPolicy) (bool, error) PruneOldImages(docker DockerClient) error UpdateImagePullRecord(path string, imageID string, timestamp string) error - UpdateImageJSONDuration(intervalStr string) error + GetDuration(p PullPolicy) time.Duration Read(path string) (*ImageJSON, error) Write(imageJSON *ImageJSON, path string) error } func intervalPolicy(options FetchOptions) bool { - return options.PullPolicy == PullWithInterval || options.PullPolicy == PullHourly || options.PullPolicy == PullDaily || options.PullPolicy == PullWeekly + return options.PullPolicy == PullHourly || options.PullPolicy == PullDaily || options.PullPolicy == PullWeekly } func NewPullPolicyManager(logger logging.Logger) ImagePullPolicyHandler { @@ -132,8 +132,8 @@ func (f *Fetcher) Fetch(ctx context.Context, name string, options FetchOptions) if err == nil || !errors.Is(err, ErrNotFound) { return img, err } - case PullWithInterval, PullDaily, PullHourly, PullWeekly: - pull, err := f.imagePullChecker.CheckImagePullInterval(name, imageJSONpath) + case PullDaily, PullHourly, PullWeekly: + pull, err := f.imagePullChecker.CheckImagePullInterval(name, imageJSONpath, options.PullPolicy) if err != nil { f.logger.Warnf("failed to check pulling interval for image %s, %s", name, err) } @@ -326,7 +326,7 @@ func (i *ImagePullPolicyManager) UpdateImagePullRecord(path string, imageID stri return nil } -func (i *ImagePullPolicyManager) CheckImagePullInterval(imageID string, path string) (bool, error) { +func (i *ImagePullPolicyManager) CheckImagePullInterval(imageID string, path string, pullPolicy PullPolicy) (bool, error) { imageJSON, err := i.Read(path) if err != nil { return false, err @@ -343,12 +343,7 @@ func (i *ImagePullPolicyManager) CheckImagePullInterval(imageID string, path str return false, errors.Wrap(err, "failed to parse image timestamp from JSON") } - durationStr := imageJSON.Interval.PullingInterval - - duration, err := parseDurationString(durationStr) - if err != nil { - return false, errors.Wrap(err, "failed to parse duration from JSON") - } + duration := i.GetDuration(pullPolicy) timeThreshold := time.Now().Add(-duration) diff --git a/pkg/image/fetcher_test.go b/pkg/image/fetcher_test.go index bedcba7015..bb3a0b621f 100644 --- a/pkg/image/fetcher_test.go +++ b/pkg/image/fetcher_test.go @@ -11,6 +11,7 @@ import ( "time" "github.com/buildpacks/imgutil" + "github.com/golang/mock/gomock" "github.com/buildpacks/imgutil/local" "github.com/buildpacks/imgutil/remote" @@ -30,7 +31,9 @@ var docker client.CommonAPIClient var logger logging.Logger var registryConfig *h.TestRegistryConfig var imageJSON *image.ImageJSON -var mockImagePullPolicyHandler = testmocks.NewMockPullPolicyManager(logger) +var mockController *gomock.Controller + +var mockImagePullPolicyHandler = testmocks.NewMockImagePullPolicyHandler(mockController) func TestFetcher(t *testing.T) { color.Disable(true) diff --git a/pkg/image/pull_policy.go b/pkg/image/pull_policy.go index 1d9a47bff4..eeb0aede0a 100644 --- a/pkg/image/pull_policy.go +++ b/pkg/image/pull_policy.go @@ -2,12 +2,8 @@ package image import ( "encoding/json" - "fmt" "os" "path/filepath" - "regexp" - "strconv" - "strings" "time" "github.com/pkg/errors" @@ -21,19 +17,17 @@ import ( // PullPolicy defines a policy for how to manage images type PullPolicy int -var interval string +const ( + PRUNE_TIME = 7 * 24 * 60 * time.Minute + HOURLY = 1 * 60 * time.Minute + DAILY = 1 * 24 * 60 * time.Minute + WEEKLY = 7 * 24 * 60 * time.Minute +) type ImagePullPolicyManager struct { Logger logging.Logger } -var ( - hourly = "1h" - daily = "1d" - weekly = "7d" - intervalRegex = regexp.MustCompile(`^(\d+d)?(\d+h)?(\d+m)?$`) -) - const ( // Always pull images, even if they are present PullAlways PullPolicy = iota @@ -47,14 +41,10 @@ const ( PullNever // PullIfNotPresent pulls images if they aren't present PullIfNotPresent - // PullWithInterval pulls images with specified intervals - PullWithInterval ) type Interval struct { - PullingInterval string `json:"pulling_interval"` - PruningInterval string `json:"pruning_interval"` - LastPrune string `json:"last_prune"` + LastPrune string `json:"last_prune"` } type ImageData struct { @@ -79,44 +69,9 @@ var nameMap = map[string]PullPolicy{"always": PullAlways, "hourly": PullHourly, // ParsePullPolicy from string with support for interval formats func (i *ImagePullPolicyManager) ParsePullPolicy(policy string) (PullPolicy, error) { if val, ok := nameMap[policy]; ok { - if val == PullHourly { - err := i.UpdateImageJSONDuration(hourly) - if err != nil { - return PullAlways, err - } - } - if val == PullDaily { - err := i.UpdateImageJSONDuration(daily) - if err != nil { - return PullAlways, err - } - } - if val == PullWeekly { - err := i.UpdateImageJSONDuration(weekly) - if err != nil { - return PullAlways, err - } - } - return val, nil } - if strings.HasPrefix(policy, "interval=") { - interval = policy - intervalStr := strings.TrimPrefix(policy, "interval=") - matches := intervalRegex.FindStringSubmatch(intervalStr) - if len(matches) == 0 { - return PullAlways, errors.Errorf("invalid interval format: %s", intervalStr) - } - - err := i.UpdateImageJSONDuration(intervalStr) - if err != nil { - return PullAlways, err - } - - return PullWithInterval, nil - } - return PullAlways, errors.Errorf("invalid pull policy %s", policy) } @@ -134,58 +89,22 @@ func (p PullPolicy) String() string { return "never" case PullIfNotPresent: return "if-not-present" - case PullWithInterval: - return fmt.Sprintf("%v", interval) } return "" } -func (i *ImagePullPolicyManager) UpdateImageJSONDuration(intervalStr string) error { - path, err := DefaultImageJSONPath() - if err != nil { - return err - } - - imageJSON, err := i.Read(path) - if err != nil { - return err - } - - imageJSON.Interval.PullingInterval = intervalStr - - return i.Write(imageJSON, path) -} - -func parseDurationString(durationStr string) (time.Duration, error) { - var totalMinutes int - for i := 0; i < len(durationStr); { - endIndex := i + 1 - for endIndex < len(durationStr) && durationStr[endIndex] >= '0' && durationStr[endIndex] <= '9' { - endIndex++ - } - - value, err := strconv.Atoi(durationStr[i:endIndex]) - if err != nil { - return 0, errors.Wrapf(err, "invalid interval format: %s", durationStr) - } - unit := durationStr[endIndex] - - switch unit { - case 'd': - totalMinutes += value * 24 * 60 - case 'h': - totalMinutes += value * 60 - case 'm': - totalMinutes += value - default: - return 0, errors.Errorf("invalid interval uniit: %s", string(unit)) - } - - i = endIndex + 1 +func (i *ImagePullPolicyManager) GetDuration(p PullPolicy) time.Duration { + switch p { + case PullHourly: + return HOURLY + case PullDaily: + return DAILY + case PullWeekly: + return WEEKLY } - return time.Duration(totalMinutes) * time.Minute, nil + return 0 } func (i *ImagePullPolicyManager) PruneOldImages(docker DockerClient) error { @@ -204,24 +123,13 @@ func (i *ImagePullPolicyManager) PruneOldImages(docker DockerClient) error { return errors.Wrap(err, "failed to parse last prune timestamp from JSON") } - pruningInterval, err := parseDurationString(imageJSON.Interval.PruningInterval) - if err != nil { - return errors.Wrap(err, "failed to parse pruning interval from JSON") - } - - if time.Since(lastPruneTime) < pruningInterval { + if time.Since(lastPruneTime) < PRUNE_TIME { // not enough time has passed since the last prune return nil } } - // prune images older than the pruning interval - pruningDuration, err := parseDurationString(imageJSON.Interval.PruningInterval) - if err != nil { - return errors.Wrap(err, "failed to parse pruning interval from JSON") - } - - pruningThreshold := time.Now().Add(-pruningDuration) + pruningThreshold := time.Now().Add(-PRUNE_TIME) for imageID, timestamp := range imageJSON.Image.ImageIDtoTIME { imageTimestamp, err := time.Parse(time.RFC3339, timestamp) @@ -256,9 +164,7 @@ func (i *ImagePullPolicyManager) Read(path string) (*ImageJSON, error) { if _, err := os.Stat(path); os.IsNotExist(err) { return &ImageJSON{ Interval: &Interval{ - PullingInterval: "7d", - PruningInterval: "7d", - LastPrune: "", + LastPrune: "", }, Image: &ImageData{}, }, nil diff --git a/pkg/image/pull_policy_test.go b/pkg/image/pull_policy_test.go index 6fe10fca6f..265c992311 100644 --- a/pkg/image/pull_policy_test.go +++ b/pkg/image/pull_policy_test.go @@ -27,7 +27,7 @@ func testPullPolicy(t *testing.T, when spec.G, it spec.S) { it.Before(func() { logger = logging.NewLogWithWriters(&outBuf, &outBuf) - imagePullChecker = fetcher_mock.NewMockPullPolicyManager(logger) + imagePullChecker = fetcher_mock.NewMockPullPolicyManager(mockController) }) it("returns PullNever for never", func() { diff --git a/pkg/testmocks/mock_image_fetcher.go b/pkg/testmocks/mock_image_fetcher.go index 90225529e2..a82a54a9dc 100644 --- a/pkg/testmocks/mock_image_fetcher.go +++ b/pkg/testmocks/mock_image_fetcher.go @@ -1,7 +1,7 @@ // Code generated by MockGen. DO NOT EDIT. // Source: github.com/buildpacks/pack/pkg/client (interfaces: ImageFetcher) -// Package testmocks is a generated GoMock package. +// Package mock_client is a generated GoMock package. package testmocks import ( @@ -9,15 +9,10 @@ import ( reflect "reflect" imgutil "github.com/buildpacks/imgutil" - gomock "github.com/golang/mock/gomock" - - "github.com/buildpacks/pack/pkg/logging" - image "github.com/buildpacks/pack/pkg/image" + gomock "github.com/golang/mock/gomock" ) -var imageJSON *image.ImageJSON - // MockImageFetcher is a mock of ImageFetcher interface. type MockImageFetcher struct { ctrl *gomock.Controller @@ -29,17 +24,6 @@ type MockImageFetcherMockRecorder struct { mock *MockImageFetcher } -type MockImagePullPolicyHandler struct { - *image.ImagePullPolicyManager - MockParsePullPolicy func(policy string, logger logging.Logger) (image.PullPolicy, error) - MockCheckImagePullInterval func(imageID string, path string) (bool, error) - MockPruneOldImages func(docker *image.DockerClient) error - MockUpdateImagePullRecord func(path string, imageID string, timestamp string) error - MockUpdateImageJSONDuration func(intervalStr string) error - MockRead func(path string) (*image.ImageJSON, error) - MockWrite func(imageJSON *image.ImageJSON, path string) error -} - // NewMockImageFetcher creates a new mock instance. func NewMockImageFetcher(ctrl *gomock.Controller) *MockImageFetcher { mock := &MockImageFetcher{ctrl: ctrl} @@ -66,66 +50,3 @@ func (mr *MockImageFetcherMockRecorder) Fetch(arg0, arg1, arg2 interface{}) *gom mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Fetch", reflect.TypeOf((*MockImageFetcher)(nil).Fetch), arg0, arg1, arg2) } - -func NewMockPullPolicyManager(logger logging.Logger) *MockImagePullPolicyHandler { - return &MockImagePullPolicyHandler{} -} - -func (m *MockImagePullPolicyHandler) CheckImagePullInterval(imageID string, path string) (bool, error) { - if m.MockCheckImagePullInterval != nil { - return m.MockCheckImagePullInterval(imageID, path) - } - return false, nil -} - -func (m *MockImagePullPolicyHandler) Write(imageJSON *image.ImageJSON, path string) error { - if m.MockWrite != nil { - return m.MockWrite(imageJSON, path) - } - return nil -} - -func (m *MockImagePullPolicyHandler) Read(path string) (*image.ImageJSON, error) { - if m.MockRead != nil { - return m.MockRead(path) - } - - imageJSON = &image.ImageJSON{ - Interval: &image.Interval{ - PullingInterval: "7d", - PruningInterval: "7d", - LastPrune: "2023-01-01T00:00:00Z", - }, - Image: &image.ImageData{ - ImageIDtoTIME: map[string]string{ - "repoName": "2023-01-01T00:00:00Z", - }, - }, - } - - return imageJSON, nil -} - -func (m *MockImagePullPolicyHandler) PruneOldImages(docker image.DockerClient) error { - if m.MockPruneOldImages != nil { - return m.MockPruneOldImages(&docker) - } - - return m.ImagePullPolicyManager.PruneOldImages(docker) -} - -func (m *MockImagePullPolicyHandler) UpdateImagePullRecord(path string, imageID string, timestamp string) error { - if m.MockUpdateImagePullRecord != nil { - return m.MockUpdateImagePullRecord(path, imageID, timestamp) - } - - return nil -} - -func (m *MockImagePullPolicyHandler) UpdateImageJSONDuration(intervalStr string) error { - if m.MockUpdateImageJSONDuration != nil { - return m.MockUpdateImageJSONDuration(intervalStr) - } - - return nil -} diff --git a/pkg/testmocks/mock_image_pull_policy_handler.go b/pkg/testmocks/mock_image_pull_policy_handler.go new file mode 100644 index 0000000000..4d72fd1e1d --- /dev/null +++ b/pkg/testmocks/mock_image_pull_policy_handler.go @@ -0,0 +1,137 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: github.com/buildpacks/pack/pkg/image (interfaces: ImagePullPolicyHandler) + +// Package mock_image is a generated GoMock package. +package testmocks + +import ( + reflect "reflect" + time "time" + + image "github.com/buildpacks/pack/pkg/image" + gomock "github.com/golang/mock/gomock" +) + +// MockImagePullPolicyHandler is a mock of ImagePullPolicyHandler interface. +type MockImagePullPolicyHandler struct { + ctrl *gomock.Controller + recorder *MockImagePullPolicyHandlerMockRecorder +} + +// MockImagePullPolicyHandlerMockRecorder is the mock recorder for MockImagePullPolicyHandler. +type MockImagePullPolicyHandlerMockRecorder struct { + mock *MockImagePullPolicyHandler +} + +// NewMockImagePullPolicyHandler creates a new mock instance. +func NewMockImagePullPolicyHandler(ctrl *gomock.Controller) *MockImagePullPolicyHandler { + mock := &MockImagePullPolicyHandler{ctrl: ctrl} + mock.recorder = &MockImagePullPolicyHandlerMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockImagePullPolicyHandler) EXPECT() *MockImagePullPolicyHandlerMockRecorder { + return m.recorder +} + +// CheckImagePullInterval mocks base method. +func (m *MockImagePullPolicyHandler) CheckImagePullInterval(arg0, arg1 string, arg2 image.PullPolicy) (bool, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CheckImagePullInterval", arg0, arg1, arg2) + ret0, _ := ret[0].(bool) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// CheckImagePullInterval indicates an expected call of CheckImagePullInterval. +func (mr *MockImagePullPolicyHandlerMockRecorder) CheckImagePullInterval(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CheckImagePullInterval", reflect.TypeOf((*MockImagePullPolicyHandler)(nil).CheckImagePullInterval), arg0, arg1, arg2) +} + +// GetDuration mocks base method. +func (m *MockImagePullPolicyHandler) GetDuration(arg0 image.PullPolicy) time.Duration { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetDuration", arg0) + ret0, _ := ret[0].(time.Duration) + return ret0 +} + +// GetDuration indicates an expected call of GetDuration. +func (mr *MockImagePullPolicyHandlerMockRecorder) GetDuration(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetDuration", reflect.TypeOf((*MockImagePullPolicyHandler)(nil).GetDuration), arg0) +} + +// ParsePullPolicy mocks base method. +func (m *MockImagePullPolicyHandler) ParsePullPolicy(arg0 string) (image.PullPolicy, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ParsePullPolicy", arg0) + ret0, _ := ret[0].(image.PullPolicy) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ParsePullPolicy indicates an expected call of ParsePullPolicy. +func (mr *MockImagePullPolicyHandlerMockRecorder) ParsePullPolicy(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ParsePullPolicy", reflect.TypeOf((*MockImagePullPolicyHandler)(nil).ParsePullPolicy), arg0) +} + +// PruneOldImages mocks base method. +func (m *MockImagePullPolicyHandler) PruneOldImages(arg0 image.DockerClient) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "PruneOldImages", arg0) + ret0, _ := ret[0].(error) + return ret0 +} + +// PruneOldImages indicates an expected call of PruneOldImages. +func (mr *MockImagePullPolicyHandlerMockRecorder) PruneOldImages(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "PruneOldImages", reflect.TypeOf((*MockImagePullPolicyHandler)(nil).PruneOldImages), arg0) +} + +// Read mocks base method. +func (m *MockImagePullPolicyHandler) Read(arg0 string) (*image.ImageJSON, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Read", arg0) + ret0, _ := ret[0].(*image.ImageJSON) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Read indicates an expected call of Read. +func (mr *MockImagePullPolicyHandlerMockRecorder) Read(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Read", reflect.TypeOf((*MockImagePullPolicyHandler)(nil).Read), arg0) +} + +// UpdateImagePullRecord mocks base method. +func (m *MockImagePullPolicyHandler) UpdateImagePullRecord(arg0, arg1, arg2 string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UpdateImagePullRecord", arg0, arg1, arg2) + ret0, _ := ret[0].(error) + return ret0 +} + +// UpdateImagePullRecord indicates an expected call of UpdateImagePullRecord. +func (mr *MockImagePullPolicyHandlerMockRecorder) UpdateImagePullRecord(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateImagePullRecord", reflect.TypeOf((*MockImagePullPolicyHandler)(nil).UpdateImagePullRecord), arg0, arg1, arg2) +} + +// Write mocks base method. +func (m *MockImagePullPolicyHandler) Write(arg0 *image.ImageJSON, arg1 string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Write", arg0, arg1) + ret0, _ := ret[0].(error) + return ret0 +} + +// Write indicates an expected call of Write. +func (mr *MockImagePullPolicyHandlerMockRecorder) Write(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Write", reflect.TypeOf((*MockImagePullPolicyHandler)(nil).Write), arg0, arg1) +}