Skip to content

feat: add separate server image tags ( #673)#706

Open
mihir-dixit2k27 wants to merge 11 commits into
uyuni-project:mainfrom
mihir-dixit2k27:feat/server-tags-refactor
Open

feat: add separate server image tags ( #673)#706
mihir-dixit2k27 wants to merge 11 commits into
uyuni-project:mainfrom
mihir-dixit2k27:feat/server-tags-refactor

Conversation

@mihir-dixit2k27

@mihir-dixit2k27 mihir-dixit2k27 commented Jan 19, 2026

Copy link
Copy Markdown

Fixes #673

Problem

The --image and --tag flags apply to all containers equally, making it impossible to pin the server container to a specific image or tag without affecting other containers (coco, pgsql, saline, etc.).

Solution

Introduce two new server-specific flags:

  • --server-image: overrides the server container image name
  • --server-tag: overrides the tag for the server container only; falls back to the global --tag if not provided

Changes

  • mgradm/shared/utils/flags.go: Added ServerImageFlags struct to ServerFlags; viper maps --server-imageserver.image and --server-tagserver.tag, following the same pattern as other container flags
  • mgradm/shared/utils/cmd_utils.go: Added AddServerImageFlags(), called from AddServerFlags() so the flags are available for install, upgrade and migration. Restored deleted comment explaining why some flags have no help group
  • mgradm/cmd/install/install.go: Wired the override in getFlagsUpdater — if Server.Image or Server.Tag is set, it takes over Image.Name/Image.Tag respectively. Restored deleted comment on the podman alias args handling
  • shared/podman/utils.go: Removed Name/Tag fields from PodmanFlags where they did not belong
  • Tests: Added ServerImageOnlyFlagsTestArgs and AssertServerImageFlags covering the full override chain with distinct values, so the test genuinely fails if the wiring is broken. Updated mgradm_install.go and server.go test helpers accordingly

Testing

  • --server-image and --server-tag appear correctly in mgradm install --help
  • Override chain verified: --server-image custom/server correctly populates Image.Name, independently of --image
  • Backward compatibility confirmed: if --server-tag is not provided, the global --tag value is used
  • Full test suite passes with zero failures

Signed-off-by: Mihir Dixit <dixitmihir1@gmail.com>
Signed-off-by: Mihir Dixit <dixitmihir1@gmail.com>
Signed-off-by: Mihir Dixit<dixitmihir1@gmail.com>
Signed-off-by : Mihir Dixit <dixitmihir1@gmail.com>

@cbosdo cbosdo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I believe that the new image tags handling is brittle…

Comment thread mgradm/shared/utils/cmd_utils.go Outdated
Comment thread mgradm/shared/utils/cmd_utils.go Outdated
Comment thread shared/podman/utils.go Outdated
Comment thread mgradm/cmd/install/install.go Outdated
Comment thread mgradm/cmd/install/install.go Outdated
Comment thread mgradm/cmd/install/install.go Outdated
Comment thread mgradm/cmd/install/install.go Outdated
@mihir-dixit2k27

Copy link
Copy Markdown
Author

@cbosdo Thanks for the guidance! I have addressed all your feedback:

Refactor: I removed the manual flag shuffling in install.go and now rely on the mapstructure tags as you suggested.

Cleanup: I removed the verbose comments in cmd_utils.go and install.go.

Conflicts: I have resolved the conflicts with the recent refactor in main.

Regarding Squashing: Since I am working through the Web UI to ensure valid GPG signatures (due to local environment restrictions), I cannot easily squash these commits locally without breaking the signatures. Could you please use the "Squash and Merge" option when merging this PR?

Ready for final review!

@sonarqubecloud

Copy link
Copy Markdown

@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
1 New issue
32.4% Duplication on New Code (required ≤ 20%)
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@cbosdo cbosdo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This PR changes a crucial part of the logic (the image URLs computation) and there is no unit test added or adjusted. This looks really strange.

Comment thread mgradm/cmd/install/install.go Outdated
}

// If the alias "install podman" is used, "podman" will be the first arg.
// We remove it from the args slice so it isn't treated as the FQDN.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why are you removing this valuable comment? I wonder how much of this PR has been generated by an AI…

_ = utils.AddFlagHelpGroup(cmd, &utils.Group{ID: "image", Title: L("Image Flags")})
_ = utils.AddFlagToHelpGroupID(cmd, "image", "image")
_ = utils.AddFlagToHelpGroupID(cmd, "tag", "image")
// without group, since this flag is applied to all the images

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why removing that comment: it explains why there is no group on those flags…

Comment thread shared/podman/utils.go Outdated
Comment on lines +55 to +56
Name string `mapstructure:"server-image"`
Tag string `mapstructure:"server-tag"`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These flags should go in the ServerFlags rather than this podman-specific struct. Someday we may remove this relic from a not so far future, but avoid mixing them for now.

@mihir-dixit2k27

Copy link
Copy Markdown
Author

@cbosdo Acknowledged — I'm working through all the review feedback carefully. Will push the fixes once everything is properly addressed.

mihir-dixit2k27 and others added 2 commits April 10, 2026 15:12
Add --server-image and --server-tag flags so users can pin the server
container image independently of the global --image and --tag flags,
which apply to all containers (coco, pgsql, saline, etc.).

- Add ServerImageFlags struct and Server field to ServerFlags; viper maps
  --server-image to server.image and --server-tag to server.tag
- Add AddServerImageFlags() in cmd_utils.go, called from AddServerFlags()
- Wire the override in getFlagsUpdater (install) and the inline updater
  (upgrade): Server.Image -> Image.Name, Server.Tag -> Image.Tag
- Move Name/Tag out of PodmanFlags; they belong in ServerFlags
- Restore deleted comment and help group lines in cmd_utils.go
- Restore deleted comment in install.go RunE
- Add AssertServerImageFlags test helper and ServerImageOnlyFlagsTestArgs
  with distinct values to verify the override chain actually runs

Signed-off-by: Mihir Dixit <dixitmihir1@gmail.com>
@sonarqubecloud

Copy link
Copy Markdown

@mihir-dixit2k27 mihir-dixit2k27 changed the title feat: add podman binary check and separate server image tags (Fixes #621, #673) feat: add separate server image tags ( #673) Apr 10, 2026
@mihir-dixit2k27 mihir-dixit2k27 requested a review from cbosdo April 10, 2026 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Separate the server tag from the global one

2 participants