[WIP] NO-JIRA | ci: remove "kubev2v" registry, use "redhat-user-workloads/assisted-migration-tenant" instead #517
Conversation
nirarg
requested review from
Jos-Jerus,
machacekondra,
ronenav and
tupyy
as code owners
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThe PR updates the image registry organization reference in the Makefile to point to the new ‘redhat-user-workloads/assisted-migration-tenant’ namespace and removes the obsolete GitHub Actions build-push-images workflow. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
WalkthroughDeleted a GitHub Actions workflow that built and pushed container images and switched default image references and docs from quay.io/kubev2v to quay.io/redhat-user-workloads/assisted-migration-tenant. No public API or exported signatures were changed. Changes
Sequence Diagram(s)(omitted — changes are configuration and delete-only, no new control flow to visualize) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
nirarg
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Makefile (1)
5-5: GO_FILES glob is incorrect; rebuilds may miss source changes-name ".go" matches only a file literally named .go. This likely prevents container rebuilds on source edits.
-GO_FILES := $(shell find ./ -name ".go" -not -path "./bin" -not -path "./packaging/*") +GO_FILES := $(shell find ./ -name "*.go" -not -path "./bin/*" -not -path "./packaging/*")
🧹 Nitpick comments (2)
Makefile (2)
156-157: Avoid hard-coding registry in loginUse $(REGISTRY) instead of quay.io to keep REGISTRY overrideable.
- $(PODMAN) login --authfile $(DOCKER_AUTH_FILE) -u=$(QUAY_USER) -p=$(QUAY_TOKEN) quay.io; \ + $(PODMAN) login --authfile $(DOCKER_AUTH_FILE) -u=$(QUAY_USER) -p=$(QUAY_TOKEN) $(REGISTRY); \
13-14: Optional: deduplicate registry path constructionMinor readability improvement by introducing REGISTRY_PATH.
+REGISTRY_PATH := $(REGISTRY)/$(REGISTRY_ORG) -MIGRATION_PLANNER_AGENT_IMAGE ?= $(REGISTRY)/$(REGISTRY_ORG)/migration-planner-agent -MIGRATION_PLANNER_API_IMAGE ?= $(REGISTRY)/$(REGISTRY_ORG)/migration-planner-api +MIGRATION_PLANNER_AGENT_IMAGE ?= $(REGISTRY_PATH)/migration-planner-agent +MIGRATION_PLANNER_API_IMAGE ?= $(REGISTRY_PATH)/migration-planner-api
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/workflows/build-push-images.yml(0 hunks)Makefile(1 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/build-push-images.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Red Hat Konflux / migration-planner-agent-on-pull-request
- GitHub Check: Red Hat Konflux / migration-planner-api-on-pull-request
- GitHub Check: unit-test
- GitHub Check: test
- GitHub Check: lint
| VERBOSE ?= false | ||
| REGISTRY ?= quay.io | ||
| REGISTRY_ORG ?= kubev2v | ||
| REGISTRY_ORG ?= redhat-user-workloads/assisted-migration-tenant |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Registry org change: confirm repo and creds exist in new namespace
Switching to redhat-user-workloads/assisted-migration-tenant looks fine. Please verify:
- The repositories for migration-planner-agent and migration-planner-api exist under this org and you have push rights.
- Any docs/automation that referenced kubev2v are updated accordingly (since the build/push workflow was removed).
Run to spot leftovers:
🏁 Script executed:
#!/bin/bash
# Find old registry/org references and deleted workflow remnants
rg -nPI 'kubev2v|quay\.io/kubev2v' -g '!*vendor*'
rg -nPI '\.github/workflows/build-push-images\.yml|Build and push images'Length of output: 26888
🏁 Script executed:
#!/bin/bash
# Locate file paths for hardcoded quay.io/kubev2v references
rg -nH 'defaultAgentImage' || true
rg -nH 'skopeo copy.*quay\.io/kubev2v' || trueLength of output: 634
Registry org change: update remaining quay.io/kubev2v references
We’ve confirmed that two hard-coded quay.io/kubev2v paths remain and must be updated to use the new $(REGISTRY_ORG) (i.e. redhat-user-workloads/assisted-migration-tenant):
- internal/image/builder.go:
- Line 35:
defaultAgentImage = "quay.io/kubev2v/migration-planner-agent"
→ should be$(REGISTRY_ORG)/migration-planner-agentinstead of the quay.io path.
- Line 35:
- data/ignition.template:
- Lines 122 & 125: skopeo copy commands targeting
containers-storage:quay.io/kubev2v/migration-planner-agent:latest(and the forklift-validation image)
→ these should reference$(REGISTRY_ORG)/migration-planner-agent:latest(and, if applicable, the new org for forklift-validation).
- Lines 122 & 125: skopeo copy commands targeting
Next steps:
- Update those constants/templates to use
$(REGISTRY_ORG) - Ensure the
migration-planner-agentandmigration-planner-apirepos exist underredhat-user-workloads/assisted-migration-tenantand that you have push rights. - Scan docs and automation for any other
quay.io/kubev2vreferences and update accordingly.
🤖 Prompt for AI Agents
In Makefile line 10 and related files: internal/image/builder.go line 35 and
data/ignition.template lines 122 and 125, there are remaining hard-coded
quay.io/kubev2v paths that must be replaced with the new REGISTRY_ORG variable;
update internal/image/builder.go to construct defaultAgentImage as
"$(REGISTRY_ORG)/migration-planner-agent" (or use the existing Go build-time var
pattern), and update the skopeo copy lines in data/ignition.template to
reference "$(REGISTRY_ORG)/migration-planner-agent:latest" (and the equivalent
for forklift-validation) instead of containers-storage:quay.io/..., then verify
the migration-planner-agent and migration-planner-api repos exist under
redhat-user-workloads/assisted-migration-tenant and you have push rights, and
finally scan docs/automation for any other quay.io/kubev2v occurrences and
replace them with $(REGISTRY_ORG).
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Makefile (1)
5-5: Bug: GO_FILES pattern matches only a file literally named “.go”.This breaks change detection for container rebuild deps. Use *.go and fix path exclusions.
-GO_FILES := $(shell find ./ -name ".go" -not -path "./bin" -not -path "./packaging/*") +GO_FILES := $(shell find . -type f -name "*.go" -not -path "./bin/*" -not -path "./packaging/*")
♻️ Duplicate comments (1)
Makefile (1)
10-10: Update remaining quay.io/kubev2v references across the repo.Past scans showed hard-coded kubev2v paths (e.g., internal/image/builder.go defaultAgentImage, data/ignition.template skopeo lines). Update them to respect
$(REGISTRY)/$ (REGISTRY_ORG).#!/bin/bash rg -nPI 'quay\.io/kubev2v|[^A-Za-z]kubev2v[^A-Za-z]'
🧹 Nitpick comments (3)
Makefile (3)
153-158: Make registry login honor $(REGISTRY).quay-login hardcodes quay.io; switch to $(REGISTRY) to keep targets portable.
- $(PODMAN) login --authfile $(DOCKER_AUTH_FILE) -u=$(QUAY_USER) -p=$(QUAY_TOKEN) quay.io; \ + $(PODMAN) login --authfile $(DOCKER_AUTH_FILE) -u=$(QUAY_USER) -p=$(QUAY_TOKEN) $(REGISTRY); \
13-14: DRY the base registry namespace.Reduce duplication and simplify future changes by factoring a base variable.
+REGISTRY_NS ?= $(REGISTRY)/$(REGISTRY_ORG) -MIGRATION_PLANNER_AGENT_IMAGE ?= $(REGISTRY)/$(REGISTRY_ORG)/migration-planner-agent -MIGRATION_PLANNER_API_IMAGE ?= $(REGISTRY)/$(REGISTRY_ORG)/migration-planner-api +MIGRATION_PLANNER_AGENT_IMAGE ?= $(REGISTRY_NS)/migration-planner-agent +MIGRATION_PLANNER_API_IMAGE ?= $(REGISTRY_NS)/migration-planner-api
61-76: Deduplicate help output.The help target prints duplicate entries (generate, tidy, lint, build, clean). Trim the second block.
@echo " setup-opa-policies: download OPA policies from Forklift project" @echo " clean-opa-policies: clean OPA policies directory" - @echo " generate: regenerate all generated files" - @echo " tidy: tidy go mod" - @echo " lint: run golangci-lint" - @echo " build: run all builds" - @echo " clean: clean up all containers and volumes" @echo " migrate: run database migrations"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/workflows/build-push-images.yml(0 hunks)Makefile(1 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/build-push-images.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Red Hat Konflux / migration-planner-api-on-pull-request
- GitHub Check: Red Hat Konflux / migration-planner-agent-on-pull-request
- GitHub Check: unit-test
- GitHub Check: test
- GitHub Check: lint
🔇 Additional comments (1)
Makefile (1)
10-14: Verify Quay repository path compositionPlease confirm that using a two-segment namespace in
REGISTRY_ORG(i.e.redhat-user-workloads/assisted-migration-tenant) is valid in Quay—this produces image names likequay.io/redhat-user-workloads/assisted-migration-tenant/migration-planner-agentQuay’s registry v2 API allows slash-separated repository names, but some organizations treat only the first segment as the namespace. Make sure that:
- The full path (
redhat-user-workloads/assisted-migration-tenant) is indeed registered as a single “repository” in Quay, not as two nested namespaces.- You have push (write) permissions for both
migration-planner-agentandmigration-planner-apiunder that path.You can verify existence (and infer permissions) by checking the manifest endpoint—for example:
curl -I https://quay.io/v2/redhat-user-workloads/assisted-migration-tenant/migration-planner-agent/manifests/latest
- A
401 Unauthorizedresponse indicates the repo exists but is private (you’ll need credentials to push).- A
404 Not Foundmeans the path doesn’t exist as given and needs correction.If this three-segment path isn’t valid, adjust
REGISTRY_ORGto the correct namespace (e.g.redhat-user-workloads) or split outassisted-migration-tenantappropriately.
…ssisted-migration-tenant" instead Signed-off-by: Nir Argaman <[email protected]>
nirarg
force-pushed
the
quay-registry
branch
from
d1ab20b to
e3a04c7
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: nirarg The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/image/builder.go (1)
445-464: Hardcoded persistence disk path/name — overrides configured value.writePersistenceDisk ignores b.PersistentDiskImage and always packs data/persistence-disk.vmdk with a fixed name. This breaks overrides set via WithPersistentDiskImage and mismatches diskSize(). Use the configured path and a header name derived from it.
@@ -import ( +import ( "archive/tar" "bytes" "context" "fmt" "io" + "path/filepath" "os" "text/template" "time" ) @@ func (b *ImageBuilder) writePersistenceDisk(tw *tar.Writer) error { - diskContent, err := os.ReadFile("data/persistence-disk.vmdk") + diskContent, err := os.ReadFile(b.PersistentDiskImage) if err != nil { return err } header := &tar.Header{ - Name: "persistence-disk.vmdk", + Name: filepath.Base(b.PersistentDiskImage), Size: int64(len(diskContent)), Mode: 0600, ModTime: time.Now(), } if err := tw.WriteHeader(header); err != nil { return err } - if _, err := tw.Write([]byte(diskContent)); err != nil { + if _, err := tw.Write(diskContent); err != nil { return err } return nil }
🧹 Nitpick comments (3)
internal/image/builder.go (2)
35-35: Registry org switch looks good; consider tagging for consistency.Elsewhere defaults use :latest. Consider aligning here to avoid surprises when comparing logs/docs.
- defaultAgentImage = "quay.io/redhat-user-workloads/assisted-migration-tenant/migration-planner-agent" + defaultAgentImage = "quay.io/redhat-user-workloads/assisted-migration-tenant/migration-planner-agent:latest"
322-327: Typo in error message."comute" → "compute".
- return -1, fmt.Errorf("failed to comute persistent disk size: %s", err) + return -1, fmt.Errorf("failed to compute persistent disk size: %s", err)doc/githubworkflows.md (1)
40-45: Fix list formatting for “Old/New Destination” and tighten wording.Current Markdown renders unevenly (flagged by grammar tools). Use a single bullet with an arrow or a nested list.
Apply:
-- **Old Destination**: `quay.io/redhat-user-workloads/assisted-migration-tenant/migration-planner-agent` - **New Destination**: `quay.io/<YOUR-QUAY-USERNAME>/migration-planner-agent` - -- **Old Destination**: `quay.io/redhat-user-workloads/assisted-migration-tenant/migration-planner-api` - **New Destination**: `quay.io/<YOUR-QUAY-USERNAME>/migration-planner-api` +• `quay.io/redhat-user-workloads/assisted-migration-tenant/migration-planner-agent` → `quay.io/<YOUR-QUAY-USERNAME>/migration-planner-agent` +• `quay.io/redhat-user-workloads/assisted-migration-tenant/migration-planner-api` → `quay.io/<YOUR-QUAY-USERNAME>/migration-planner-api`
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
.github/workflows/build-push-images.yml(0 hunks)Makefile(1 hunks)data/ignition.template(1 hunks)doc/cli.md(2 hunks)doc/deployment.md(2 hunks)doc/githubworkflows.md(3 hunks)doc/troubleshooting.md(1 hunks)internal/cli/generate.go(2 hunks)internal/image/builder.go(1 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/build-push-images.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- Makefile
🧰 Additional context used
🪛 LanguageTool
doc/githubworkflows.md
[grammar] ~40-~40: There might be a mistake here.
Context: ... be as follows: - Old Destination: quay.io/redhat-user-workloads/assisted-migration-tenant/migration-planner-agent New Destination: `quay.io/<YOUR-QUAY-U...
(QB_NEW_EN)
[grammar] ~43-~43: There might be a mistake here.
Context: ...-planner-agent - **Old Destination**:quay.io/redhat-user-workloads/assisted-migration-tenant/migration-planner-api **New Destination**:quay.io/<YOUR-QUAY-U...
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Red Hat Konflux / migration-planner-api-on-pull-request
- GitHub Check: Red Hat Konflux / migration-planner-agent-on-pull-request
- GitHub Check: unit-test
- GitHub Check: lint
- GitHub Check: check-generate
- GitHub Check: test
🔇 Additional comments (9)
doc/cli.md (2)
168-168: EOF newline fix — LGTM.
82-82: Default image updated; no quay.io/kubev2v references remain — verified no occurrences ofquay.io/kubev2v/migration-planner-agent.data/ignition.template (1)
121-123: Skopeo destination switched to new org — LGTM.forklift-validation remains under kubev2v as expected.
doc/troubleshooting.md (1)
48-48: Sample output updated to new org — LGTM.internal/cli/generate.go (2)
31-32: Default AgentImageURL updated — LGTM.In sync with docs and template.
104-104: Flag default updated — LGTM.Help text and default are consistent.
doc/deployment.md (2)
5-5: Update CI/publishing statement (workflow was removed in this PR).This line claims images are built and pushed after each PR merge, but
.github/workflows/build-push-images.ymlwas deleted. Either remove the claim or point to the current publishing process.Apply:
-By default, images are deployed from the `quay.io/redhat-user-workloads/assisted-migration-tenant` namespace. New images are built and pushed to Quay after each PR is merged into this repository. +By default, images are deployed from the `quay.io/redhat-user-workloads/assisted-migration-tenant` namespace.
27-31: Deployment doc defaults are correct; add clarification for tag usage.The defaults in doc/deployment.md match the Makefile and code (
MIGRATION_PLANNER_API_IMAGE/MIGRATION_PLANNER_AGENT_IMAGEdefault to quay.io/redhat-user-workloads/assisted-migration-tenant/* andMIGRATION_PLANNER_IMAGE_TAGdefaults to latest). Add a one-liner below the snippet noting thatMIGRATION_PLANNER_IMAGE_TAGapplies to both API and agent images, and the UI image usesMIGRATION_PLANNER_UI_IMAGE_TAG.doc/githubworkflows.md (1)
231-231: EOF rule change OK.Separator change to
---is harmless; no action needed.
| `MIGRATION_PLANNER_AGENT_IMAGE ?= quay.io/redhat-user-workloads/assisted-migration-tenant/migration-planner-agent` | ||
| `MIGRATION_PLANNER_API_IMAGE ?= quay.io/redhat-user-workloads/assisted-migration-tenant/migration-planner-api` |
There was a problem hiding this comment.
Section likely stale: workflow was removed in this PR.
This whole “build-push-images.yml” section documents a workflow that no longer exists. Either mark it as deprecated with the removal date (Aug 27, 2025) and point to the new process, or remove the section.
If keeping the section for historical context, prepend a banner:
+# Note: The `build-push-images.yml` workflow was removed on 2025-08-27.
+# The instructions below are deprecated; see current release/publish docs or .github/workflows.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| `MIGRATION_PLANNER_AGENT_IMAGE ?= quay.io/redhat-user-workloads/assisted-migration-tenant/migration-planner-agent` | |
| `MIGRATION_PLANNER_API_IMAGE ?= quay.io/redhat-user-workloads/assisted-migration-tenant/migration-planner-api` | |
| # Note: The `build-push-images.yml` workflow was removed on 2025-08-27. | |
| # The instructions below are deprecated; see current release/publish docs or .github/workflows. | |
| `MIGRATION_PLANNER_AGENT_IMAGE ?= quay.io/redhat-user-workloads/assisted-migration-tenant/migration-planner-agent` | |
| `MIGRATION_PLANNER_API_IMAGE ?= quay.io/redhat-user-workloads/assisted-migration-tenant/migration-planner-api` |
🤖 Prompt for AI Agents
In doc/githubworkflows.md around lines 65-66, the "build-push-images.yml"
workflow section is stale because the workflow was removed in this PR; either
remove the entire section referencing MIGRATION_PLANNER_AGENT_IMAGE and
MIGRATION_PLANNER_API_IMAGE, or mark it explicitly as deprecated by prepending a
short banner that states "Deprecated (removed Aug 27, 2025)" and add one
sentence pointing readers to the new build/push process (include the new
workflow file or docs link and any changed variables), and update or remove the
two image variable lines accordingly so the doc no longer advertises an obsolete
workflow.
|
PR needs rebase. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/retest |
3 participants
Summary by Sourcery
Update default container registry organization to redhat-user-workloads/assisted-migration-tenant and remove the obsolete build-push-images GitHub Actions workflow.
Enhancements:
CI:
Summary by CodeRabbit