Skip to content

Commit

Permalink
refactor: Refactor image JSON handling into a dedicated manager
Browse files Browse the repository at this point in the history
- 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 <parthibahazra@gmail.com>
  • Loading branch information
Parthiba-Hazra committed Mar 10, 2024
1 parent 0bb8f69 commit f8740a4
Show file tree
Hide file tree
Showing 14 changed files with 172 additions and 181 deletions.
2 changes: 1 addition & 1 deletion internal/commands/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,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)
}
Expand Down
2 changes: 1 addition & 1 deletion internal/commands/builder_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion internal/commands/buildpack_package.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
26 changes: 12 additions & 14 deletions internal/commands/config_prune_interval.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package commands

import (
"encoding/json"
"fmt"
"regexp"

Expand All @@ -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",
Expand All @@ -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
}
Expand All @@ -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
}
Expand All @@ -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
}
Expand Down
6 changes: 3 additions & 3 deletions internal/commands/config_pull_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,15 @@ 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
}

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
}
Expand All @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion internal/commands/create_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion internal/commands/extension_package.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
2 changes: 1 addition & 1 deletion internal/commands/package_buildpack.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
2 changes: 1 addition & 1 deletion internal/commands/rebase.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}

Expand Down
67 changes: 22 additions & 45 deletions pkg/image/fetcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand All @@ -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)
}
Expand All @@ -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
}
}
Expand Down Expand Up @@ -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
}
Expand All @@ -318,37 +316,16 @@ 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
}

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
}
Expand Down
Loading

0 comments on commit f8740a4

Please sign in to comment.