changing value of DefaultInfraImage #909

noracenofun · 2022-01-26T16:52:42Z

because of issue containers/podman #12771

the new default image in 4.0.0-rc2 is a locally built one based on a local pause binary, build result e.g. "localhost/podman-pause:4.0.0-rc2-1642795764" and no longer the image "k8s.gcr.io/pause:3.5", but the command podman pod create --help does not yet support this feature for the output of the default infra-image

simplest solution - changing value of DefaultInfraImage

before correction

$ podman pod create --help | grep infra-image
      --infra-image string            The image of the infra container to associate with the pod (default "k8s.gcr.io/pause:3.5")

should be like this after correction

$ podman pod create --help | grep infra-image
      --infra-image string            The image of the infra container to associate with the pod (default "localhost/podman-pause:Version-Built")

openshift-ci · 2022-01-26T16:52:45Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: noracenofun
To complete the pull request process, please assign baude after the PR has been reviewed.
You can assign the PR to them by writing /assign @baude in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

rhatdan · 2022-01-26T18:07:24Z

pkg/config/default.go

@@ -46,7 +46,7 @@ var (
 	// DefaultInitPath is the default path to the container-init binary
 	DefaultInitPath = "/usr/libexec/podman/catatonit"
 	// DefaultInfraImage to use for infra container
-	DefaultInfraImage = "k8s.gcr.io/pause:3.5"
+	DefaultInfraImage = "localhost/podman-pause:Version-Built"


Please remove :Version-Built
Does this work when you attempt to use it?

Is only a string, as said, the simplest solution

But if this default is used somewhere it will fail.

But it should not be.

pod_create.go :: func pullOrBuildInfraImage

... if imageName != config.DefaultInfraImage { _, err := rt.LibimageRuntime().Pull(context.Background(), imageName, config.PullPolicyMissing, nil) if err != nil { return err } } else { name, err := buildPauseImage(rt, rtConfig) if err != nil { return fmt.Errorf("building local pause image: %w", err) } imageName = name } ...

I think the default should be ""
Then if the user does not enter an imageName, it will use buildPauseImage.

The info of the default image would disappear, but also a little user experience and the output should than be

$ podman pod create --help | grep infra-image --infra-image string The image of the infra container to associate with the pod

Change the help to state
Image to use for infra container, rather then builtin.

This would be a change of containers/podman/.../pods/create.go in line 78. Can you do this? I hope.

rhatdan · 2022-01-26T18:07:51Z

@Luap99 @vrothberg PTAL

rhatdan · 2022-01-26T18:24:52Z

Please squash the commits
git rebase -i origin
git push --force

the new default image in 4.0.0-rc2 is a locally built one based on a local pause binary, build result e.g. "localhost/podman-pause:4.0.0-rc2-1642795764" and no longer the image "k8s.gcr.io/pause:3.5", but the command `podman pod create --help` does not yet support this feature for the output of the default infra-image changing value of DefaultInfraImage update string ":Version-Built" removed Signed-off-by: noracenofun <[email protected]> changing value of DefaultInfraImage final default value removed Signed-off-by: noracenofun <[email protected]>

vrothberg

This is a breaking change. Podman semantics rely on that behavior:

The local pause image is only built/used by Podman if the specified infra-image != config.DefaultInfraImage.
Setting it to "" will now yield building the local image even if the user specified a custom image.

I am against merging this PR and rather fix the help messages in Podman.

noracenofun · 2022-01-27T10:02:08Z

The local pause image is only built/used by Podman if the specified infra-image != config.DefaultInfraImage.

Setting it to "" will now yield building the local image even if the user specified a custom image.

This is not quite correct.

4.0.0-rc2

$ podman version
Client:       Podman Engine
Version:      4.0.0-rc2
API Version:  4.0.0-rc2
Go Version:   go1.18beta1

Built:      Fri Jan 21 21:09:24 2022
OS/Arch:    linux/arm64

no default configuration of infra_image in any containers.conf

$ podman pod create --help | grep infra-image
      --infra-image string            The image of the infra container to associate with the pod (default "k8s.gcr.io/pause:3.5")

# default "k8s.gcr.io/pause:3.5" definition in default.go via DefaultInfraImage

$ podman images
REPOSITORY  TAG         IMAGE ID    CREATED     SIZE

$ podman pod create
765b3f7f1c22eb11d7d00a75acbd985aed36226f0d50b7c405d38ce08b0f684c

$ podman images
REPOSITORY              TAG                   IMAGE ID      CREATED        SIZE
localhost/podman-pause  4.0.0-rc2-1642795764  7c3db3338e49  8 seconds ago  622 kB

==> result: a local image was built and the default configuration of DefaultInfraImage in default.go was ignored

This shows that if there is no default configuration of infra_image in a containers.conf the current situation is

DefaultInfraImage = "k8s.gcr.io/pause:3.5"

if the specified infra-image != config.DefaultInfraImage

"k8s.gcr.io/pause:3.5" != "k8s.gcr.io/pause:3.5"

==> result: building the local image

and the future constellation would be

DefaultInfraImage = ""

if the specified infra-image != config.DefaultInfraImage

"" != ""

==> result: building the local image

So setting it to "" would be the same result, and setting infra-image via option --infra-image or containers.conf would end in a pull.

--infra-image=foobar

DefaultInfraImage = ""

if the specified infra-image != config.DefaultInfraImage

"foobar" != ""

==> result: pulling the image foobar

$ podman pod create --help | grep infra-image
      --infra-image string            The image of the infra container to associate with the pod (default "k8s.gcr.io/pause:3.5")

# default "k8s.gcr.io/pause:3.5" definition in default.go via DefaultInfraImage

$ podman images
REPOSITORY  TAG         IMAGE ID    CREATED     SIZE

$ podman pod create --infra-image registry.access.redhat.com/ubi8/pause
3599284d58eea18d63037879f5d041b8e7d4462105314bfb48a8d9b4482ea5e5

$ podman images
REPOSITORY                             TAG         IMAGE ID      CREATED      SIZE
registry.access.redhat.com/ubi8/pause  latest      5e78f220d29d  5 weeks ago  3.42 MB

This means that DefaultInfraImage is currently used only for deciding whether to built a local image or use the image configured via the --infra--image option or containers.conf.
Images configured via DefaultInfraImage in default.go are no longer pulled.

Another suggestion instead of ""

DefaultInfraImage="builtin"

vrothberg · 2022-01-27T12:13:12Z

This means that DefaultInfraImage is currently used only for deciding whether to built a local image or use the image configured via the --infra--image option or containers.conf.

Yes, that's exactly what I meant above.

Images configured via DefaultInfraImage in default.go are no longer pulled.

Yes, because that is the hard-coded default.

What's your goal? Do you want to pull the k8s pause image or do you want to correct the output of the --help message?

rhatdan · 2022-01-27T12:27:19Z

Yes, the goal should be if the user does not specify an image to use, we use the local image. Similarly if the containers.conf does not specify a image to pull we build the local image.

So help should be specify pause image to use, rather then the built in image.

BTW, almost no one will override.

noracenofun · 2022-01-27T12:46:24Z

This assuage is not correct.

Setting it to "" will now yield building the local image even if the user specified a custom image.

The opposite is the case

Setting it to "" (or any other value) will now yield building the local image if the user specified not a custom image.

My goal: Eliminate user confusion due to incorrect behavior of the default configuration via DefaultInfraImage.

So the output of the --help message should be correct.

However, if DefaultInfraImage is set by a value, it would be used by spf13/pflag/flag.go (line 724) for the output in the help message.

		if !flag.defaultIsZeroValue() {
			if flag.Value.Type() == "string" {
				line += fmt.Sprintf(" (default %q)", flag.DefValue)
			} else {
				line += fmt.Sprintf(" (default %s)", flag.DefValue)
			}
		}

vrothberg · 2022-01-27T12:52:49Z

Please change the defaults in cmd/podman rather than the defaults here. The proposed change here as impacts on the behavior and I do not think that's what we should do if all we want is to change the help message.

However, if DefaultInfraImage is set by a value, it would be used by spf13/pflag/flag.go (line 724) for the output in the help message.

That can easily be changed in the help message. Instead of calling it default, the message should describe it as a "custom" infra image that is pulled from the registry.

noracenofun mentioned this pull request Jan 26, 2022

podman pod create command ignored the default pod infra image specified in containers.conf containers/podman#12771

Closed

openshift-ci bot assigned baude and unassigned baude Jan 26, 2022

rhatdan reviewed Jan 26, 2022

View reviewed changes

vrothberg requested changes Jan 27, 2022

View reviewed changes

noracenofun closed this Jan 27, 2022

noracenofun deleted the patch-1 branch January 27, 2022 12:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

changing value of DefaultInfraImage #909

changing value of DefaultInfraImage #909

noracenofun commented Jan 26, 2022

openshift-ci bot commented Jan 26, 2022

rhatdan Jan 26, 2022

noracenofun Jan 26, 2022

rhatdan Jan 26, 2022

noracenofun Jan 26, 2022

rhatdan Jan 26, 2022

noracenofun Jan 26, 2022

rhatdan Jan 26, 2022

noracenofun Jan 26, 2022

rhatdan commented Jan 26, 2022

rhatdan commented Jan 26, 2022

vrothberg left a comment

noracenofun commented Jan 27, 2022 •

edited

Loading

vrothberg commented Jan 27, 2022

rhatdan commented Jan 27, 2022

noracenofun commented Jan 27, 2022

vrothberg commented Jan 27, 2022

changing value of DefaultInfraImage #909

changing value of DefaultInfraImage #909

Conversation

noracenofun commented Jan 26, 2022

openshift-ci bot commented Jan 26, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rhatdan commented Jan 26, 2022

rhatdan commented Jan 26, 2022

vrothberg left a comment

Choose a reason for hiding this comment

noracenofun commented Jan 27, 2022 • edited Loading

vrothberg commented Jan 27, 2022

rhatdan commented Jan 27, 2022

noracenofun commented Jan 27, 2022

vrothberg commented Jan 27, 2022

noracenofun commented Jan 27, 2022 •

edited

Loading