Skip to content
This repository was archived by the owner on Mar 16, 2024. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion integration/build/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ func TestBuildNestedAcornWithLocalImage(t *testing.T) {
}

// build the Nginx image
source := imagesource.NewImageSource("./testdata/nested/nginx.Acornfile", []string{}, []string{}, []string{})
source := imagesource.NewImageSource("./testdata/nested/nginx.Acornfile", []string{}, []string{}, []string{}, false)
image, _, err := source.GetImageAndDeployArgs(helper.GetCTX(t), c)
if err != nil {
t.Fatal(err)
Expand Down
2 changes: 1 addition & 1 deletion integration/dev/dev_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func TestDev(t *testing.T) {
eg := errgroup.Group{}
eg.Go(func() error {
return dev.Dev(subCtx, helper.BuilderClient(t, ns.Name), &dev.Options{
ImageSource: imagesource.NewImageSource(acornCueFile, []string{tmp}, nil, nil),
ImageSource: imagesource.NewImageSource(acornCueFile, []string{tmp}, nil, nil, false),
Run: client.AppRunOptions{
Name: "test-app",
},
Expand Down
53 changes: 53 additions & 0 deletions integration/run/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"github.com/acorn-io/runtime/pkg/appdefinition"
"github.com/acorn-io/runtime/pkg/client"
"github.com/acorn-io/runtime/pkg/config"
"github.com/acorn-io/runtime/pkg/imagesource"
kclient "github.com/acorn-io/runtime/pkg/k8sclient"
"github.com/acorn-io/runtime/pkg/labels"
"github.com/acorn-io/runtime/pkg/run"
Expand Down Expand Up @@ -1698,3 +1699,55 @@ func TestAutoUpgradeImageValidation(t *testing.T) {
}
assert.ErrorContains(t, err, "could not find local image for myimage:latest - if you are trying to use a remote image, specify the full registry")
}

func TestAutoUpgradeLocalImage(t *testing.T) {
ctx := helper.GetCTX(t)

helper.StartController(t)
restConfig, err := restconfig.New(scheme.Scheme)
if err != nil {
t.Fatal("error while getting rest config:", err)
}
kclient := helper.MustReturn(kclient.Default)
project := helper.TempProject(t, kclient)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😄


c, err := client.New(restConfig, project.Name, project.Name)
if err != nil {
t.Fatal(err)
}

// Attempt to run an auto-upgrade app with a non-existent local image. Should get an error.
_, err = c.AppRun(ctx, "mylocalimage", &client.AppRunOptions{
AutoUpgrade: &[]bool{true}[0],
})
if err == nil {
t.Fatalf("expected to get a not found error, instead got %v", err)
}
assert.ErrorContains(t, err, "could not find local image for mylocalimage - if you are trying to use a remote image, specify the full registry")

// Next, build the local image
image, err := c.AcornImageBuild(ctx, "./testdata/named/Acornfile", &client.AcornImageBuildOptions{})
if err != nil {
t.Fatal(err)
}

// Tag the image
err = c.ImageTag(ctx, image.ID, "mylocalimage")
if err != nil {
t.Fatal(err)
}

// Deploy the app
imageSource := imagesource.NewImageSource("", []string{"mylocalimage"}, []string{}, nil, true)
appImage, _, err := imageSource.GetImageAndDeployArgs(ctx, c)
if err != nil {
t.Fatal(err)
}

_, err = c.AppRun(ctx, appImage, &client.AppRunOptions{
AutoUpgrade: &[]bool{true}[0],
})
if err != nil {
t.Fatal(err)
}
}
2 changes: 2 additions & 0 deletions pkg/apis/api.acorn.io/v1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,8 @@ type ImageDetails struct {
DeployArgs v1.GenericMap `json:"deployArgs,omitempty"`
Profiles []string `json:"profiles,omitempty"`
Auth *RegistryAuth `json:"auth,omitempty"`
// NoDefaultRegistry - if true, do not assume a default registry on the image if none is specified
NoDefaultRegistry bool `json:"noDefaultRegistry,omitempty"`

// Output Params
AppImage v1.AppImage `json:"appImage,omitempty"`
Expand Down
30 changes: 14 additions & 16 deletions pkg/apis/internal.acorn.io/v1/appinstance.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,22 +172,20 @@ func (a AppInstanceStatus) GetDevMode() bool {
}

type AppInstanceStatus struct {
DevSession *DevSessionInstanceSpec `json:"devSession,omitempty"`
ObservedGeneration int64 `json:"observedGeneration,omitempty"`
ObservedImageDigest string `json:"observedImageDigest,omitempty"`
Columns AppColumns `json:"columns,omitempty"`
Ready bool `json:"ready,omitempty"`
Namespace string `json:"namespace,omitempty"`
AppImage AppImage `json:"appImage,omitempty"`
AvailableAppImage string `json:"availableAppImage,omitempty"`
AvailableAppImageRemote bool `json:"availableAppImageRemote,omitempty"`
ConfirmUpgradeAppImage string `json:"confirmUpgradeAppImage,omitempty"`
ConfirmUpgradeAppImageRemote bool `json:"confirmUpgradeAppImageRemote,omitempty"`
AppSpec AppSpec `json:"appSpec,omitempty"`
AppStatus AppStatus `json:"appStatus,omitempty"`
Scheduling map[string]Scheduling `json:"scheduling,omitempty"`
Conditions []Condition `json:"conditions,omitempty"`
Defaults Defaults `json:"defaults,omitempty"`
DevSession *DevSessionInstanceSpec `json:"devSession,omitempty"`
ObservedGeneration int64 `json:"observedGeneration,omitempty"`
ObservedImageDigest string `json:"observedImageDigest,omitempty"`
Columns AppColumns `json:"columns,omitempty"`
Ready bool `json:"ready,omitempty"`
Namespace string `json:"namespace,omitempty"`
AppImage AppImage `json:"appImage,omitempty"`
AvailableAppImage string `json:"availableAppImage,omitempty"`
ConfirmUpgradeAppImage string `json:"confirmUpgradeAppImage,omitempty"`
AppSpec AppSpec `json:"appSpec,omitempty"`
AppStatus AppStatus `json:"appStatus,omitempty"`
Scheduling map[string]Scheduling `json:"scheduling,omitempty"`
Conditions []Condition `json:"conditions,omitempty"`
Defaults Defaults `json:"defaults,omitempty"`
}

type Defaults struct {
Expand Down
7 changes: 0 additions & 7 deletions pkg/autoupgrade/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,6 @@ func (d *daemon) refreshImages(ctx context.Context, apps map[kclient.ObjectKey]v
// This satisfies the usecase of autoUpgrade with an app's tag is something static, like "latest"
// However, if the tag is a pattern and the current image has no tag, we don't want to check for a digest because this would
// result in a digest upgrade even though no tag matched.
var remote bool
if !updated && (!isPattern || current.Identifier() != "") {
nextAppImage = imageKey.image
var pullErr error
Expand All @@ -222,8 +221,6 @@ func (d *daemon) refreshImages(ctx context.Context, apps map[kclient.ObjectKey]v
if localDigest, ok, _ := d.client.resolveLocalTag(ctx, app.Namespace, imageKey.image); ok && localDigest != "" {
digest = localDigest
}
} else {
remote = true
}

if digest == "" && pullErr != nil {
Expand Down Expand Up @@ -252,18 +249,14 @@ func (d *daemon) refreshImages(ctx context.Context, apps map[kclient.ObjectKey]v
continue
}
app.Status.AvailableAppImage = nextAppImage
app.Status.AvailableAppImageRemote = remote
app.Status.ConfirmUpgradeAppImage = ""
app.Status.ConfirmUpgradeAppImageRemote = false
case "notify":
if app.Status.ConfirmUpgradeAppImage == nextAppImage {
d.appKeysPrevCheck[appKey] = updateTime
continue
}
app.Status.ConfirmUpgradeAppImage = nextAppImage
app.Status.ConfirmUpgradeAppImageRemote = remote
app.Status.AvailableAppImage = ""
app.Status.AvailableAppImageRemote = false
default:
logrus.Warnf("Unrecognized auto-upgrade mode %v for %v", mode, app.Name)
continue
Expand Down
2 changes: 1 addition & 1 deletion pkg/cli/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func (s *Build) Run(cmd *cobra.Command, args []string) error {
return err
}

helper := imagesource.NewImageSource(s.File, args, s.Profile, s.Platform)
helper := imagesource.NewImageSource(s.File, args, s.Profile, s.Platform, false)
image, _, err := helper.GetImageAndDeployArgs(cmd.Context(), c)
if err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion pkg/cli/dev.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func (s *Dev) Run(cmd *cobra.Command, args []string) error {
return err
}

imageSource := imagesource.NewImageSource(s.File, args, s.Profile, nil)
imageSource := imagesource.NewImageSource(s.File, args, s.Profile, nil, s.AutoUpgrade != nil && *s.AutoUpgrade)

opts, err := s.ToOpts()
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion pkg/cli/render.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func (s *Render) Run(cmd *cobra.Command, args []string) error {
return err
}

imageAndArgs := imagesource.NewImageSource(s.File, args, s.Profile, nil)
imageAndArgs := imagesource.NewImageSource(s.File, args, s.Profile, nil, false)

appDef, _, err := imageAndArgs.GetAppDefinition(cmd.Context(), c)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion pkg/cli/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ func (s *Run) Run(cmd *cobra.Command, args []string) (err error) {
}

var (
imageSource = imagesource.NewImageSource(s.File, args, s.Profile, nil)
imageSource = imagesource.NewImageSource(s.File, args, s.Profile, nil, s.AutoUpgrade != nil && *s.AutoUpgrade)
app *apiv1.App
updated bool
)
Expand Down
2 changes: 2 additions & 0 deletions pkg/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,8 @@ type ImageDetailsOptions struct {
Profiles []string
DeployArgs map[string]any
Auth *apiv1.RegistryAuth
// NoDefaultRegistry - if true, indicates that no default container registry should be assumed when getting image details
NoDefaultRegistry bool
}

type ImageDeleteOptions struct {
Expand Down
1 change: 1 addition & 0 deletions pkg/client/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ func (c *DefaultClient) ImageDetails(ctx context.Context, imageName string, opts
detailsResult.Profiles = opts.Profiles
detailsResult.NestedDigest = opts.NestedDigest
detailsResult.Auth = opts.Auth
detailsResult.NoDefaultRegistry = opts.NoDefaultRegistry
}

err := c.RESTClient.Post().
Expand Down
4 changes: 2 additions & 2 deletions pkg/controller/appdefinition/pullappimage.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,14 +55,14 @@ func pullAppImage(transport http.RoundTripper, client pullClient) router.Handler
return nil
}

// Skip the attempt to locally resolve if we already know that the image will be remote
var (
_, autoUpgradeOn = autoupgrade.Mode(appInstance.Spec)
resolved string
err error
isLocal bool
)
if !appInstance.Status.AvailableAppImageRemote {
// Only attempt to resolve locally if auto-upgrade is not on, or if auto-upgrade is on but we know the image is not remote.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment makes code easier to understand 👍

if !autoUpgradeOn || !images.IsImageRemote(target, true, remote.WithTransport(transport)) {
resolved, isLocal, err = client.resolve(req.Ctx, req.Client, appInstance.Namespace, target)
if err != nil {
cond.Error(err)
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/appdefinition/pullappimage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ func testRecordPullEvent(t *testing.T, testName string, appInstance *v1.AppInsta
return nil
}

handler := pullAppImage(nil, pullClient{
handler := pullAppImage(mockRoundTripper{}, pullClient{
recorder: event.RecorderFunc(fakeRecorder),
resolve: resolve,
pull: pull,
Expand Down
4 changes: 2 additions & 2 deletions pkg/imagedetails/imagedetails.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (
kclient "sigs.k8s.io/controller-runtime/pkg/client"
)

func GetImageDetails(ctx context.Context, c kclient.Client, namespace, imageName string, profiles []string, deployArgs map[string]any, nested string, opts ...remote.Option) (*apiv1.ImageDetails, error) {
func GetImageDetails(ctx context.Context, c kclient.Client, namespace, imageName string, profiles []string, deployArgs map[string]any, nested string, noDefaultReg bool, opts ...remote.Option) (*apiv1.ImageDetails, error) {
imageName = strings.ReplaceAll(imageName, "+", "/")
name := strings.ReplaceAll(imageName, "/", "+")

Expand All @@ -43,7 +43,7 @@ func GetImageDetails(ctx context.Context, c kclient.Client, namespace, imageName
err := c.Get(ctx, router.Key(namespace, name), image)
if err != nil && !apierror.IsNotFound(err) {
return nil, err
} else if err != nil && apierror.IsNotFound(err) && tags.IsLocalReference(name) {
} else if err != nil && apierror.IsNotFound(err) && (tags.IsLocalReference(name) || (noDefaultReg && tags.HasNoSpecifiedRegistry(imageName))) {
return nil, err
} else if err == nil {
namespace = image.Namespace
Expand Down
23 changes: 23 additions & 0 deletions pkg/images/operations.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,29 @@ func ResolveTag(tag imagename.Reference, image string) string {
return image
}

// IsImageRemote checks the remote registry to see if the given image name exists.
// If noDefaultRegistry is true, and the image does not have a specified registry, this function will return false
// without attempting to check any remote registries.
func IsImageRemote(image string, noDefaultRegistry bool, opts ...remote.Option) bool {
var (
ref imagename.Reference
err error
)
if noDefaultRegistry {
ref, err = imagename.ParseReference(image, imagename.WithDefaultRegistry(NoDefaultRegistry))
} else {
ref, err = imagename.ParseReference(image)
}

if err != nil || ref.Context().RegistryStr() == NoDefaultRegistry {
return false
}

_, err = remote.Index(ref, opts...)

return err == nil
}

func pullIndex(tag imagename.Reference, opts []remote.Option) (*v1.AppImage, error) {
img, err := remote.Index(tag, opts...)
if err != nil {
Expand Down
12 changes: 10 additions & 2 deletions pkg/imagesource/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"strings"

"github.com/acorn-io/runtime/pkg/appdefinition"
"github.com/acorn-io/runtime/pkg/autoupgrade"
"github.com/acorn-io/runtime/pkg/build"
"github.com/acorn-io/runtime/pkg/client"
"github.com/acorn-io/runtime/pkg/config"
Expand All @@ -21,13 +22,20 @@ type ImageSource struct {
Args []string
Profiles []string
Platforms []string
// NoDefaultRegistry - if true, indicates that no container registry should be assumed for the Image.
// This is used if the ImageSource is for an app with auto-upgrade enabled.
NoDefaultRegistry bool
}

func NewImageSource(file string, args, profiles, platforms []string) (result ImageSource) {
func NewImageSource(file string, args, profiles, platforms []string, noDefaultReg bool) (result ImageSource) {
result.File = file
result.Image, result.Args = splitImageAndArgs(args)
result.Profiles = profiles
result.Platforms = platforms

// If the image is a pattern, auto-upgrade is on, so assume no default registry
_, isPattern := autoupgrade.AutoUpgradePattern(result.Image)
result.NoDefaultRegistry = noDefaultReg || isPattern
return
}

Expand Down Expand Up @@ -64,7 +72,7 @@ func (i ImageSource) GetAppDefinition(ctx context.Context, c client.Client) (*ap
)
if file == "" {
sourceName = image
imageDetails, err := c.ImageDetails(ctx, image, nil)
imageDetails, err := c.ImageDetails(ctx, image, &client.ImageDetailsOptions{NoDefaultRegistry: i.NoDefaultRegistry})
if err != nil {
return nil, nil, err
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/imagesource/platforms_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ func TestParamsHelp(t *testing.T) {
"--i-default=3",
"--complex",
"@testdata/params/test.cue",
}, nil, nil).GetAppDefinition(context.Background(), nil)
}, nil, nil, false).GetAppDefinition(context.Background(), nil)
assert.Equal(t, pflag.ErrHelp, err)
}

Expand All @@ -42,7 +42,7 @@ func TestParams(t *testing.T) {
"--i-default=3",
"--complex",
"@testdata/params/test.cue",
}, nil, nil).GetAppDefinition(context.Background(), nil)
}, nil, nil, false).GetAppDefinition(context.Background(), nil)
if err != nil {
t.Fatal(err)
}
Expand Down
19 changes: 7 additions & 12 deletions pkg/openapi/generated/openapi_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion pkg/server/registry/apigroups/acorn/apps/confirmupgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ func (s *ConfirmUpgradeStrategy) Create(ctx context.Context, obj types.Object) (
return nil, err
}
app.Status.AvailableAppImage = app.Status.ConfirmUpgradeAppImage
app.Status.AvailableAppImageRemote = app.Status.ConfirmUpgradeAppImageRemote

err = s.client.Status().Update(ctx, app)
if err != nil {
Expand Down
Loading