Skip to content

Add NoRegistryClusterInstall capability to variant registry#3772

Merged
openshift-merge-bot[bot] merged 1 commit into
openshift:mainfrom
sadasu:add-agent-iri-capability
Jul 14, 2026
Merged

Add NoRegistryClusterInstall capability to variant registry#3772
openshift-merge-bot[bot] merged 1 commit into
openshift:mainfrom
sadasu:add-agent-iri-capability

Conversation

@sadasu

@sadasu sadasu commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

This change adds Component and Capability tracking for Agent IRI No registry nstaller jobs in the variant registry system.

Jobs matching the pattern '-iso-no-registry` are now classified with:

  • Component: Installer
  • Capability: NoRegistryClusterInstall
  • JobTier: candidate

This enables Sippy to track these jobs separately via the capability-based variant system introduced in PR #3696.

Summary by CodeRabbit

  • New Features

    • Added support for identifying no-registry cluster installation jobs and classifying them as installer-related.
    • Updated variant metadata to include the NoRegistryClusterInstall capability alongside the installer/agent component.
  • Tests

    • Added coverage for no-registry, agent-based compact installation jobs and the derived variant attributes, including component/capability matching.

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: automatic mode

@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 6e068da8-4210-4b53-b296-0f0dfdaf713d

📥 Commits

Reviewing files that changed from the base of the PR and between 48528d7 and 984250c.

📒 Files selected for processing (3)
  • pkg/variantregistry/ocp.go
  • pkg/variantregistry/ocp_test.go
  • pkg/variantregistry/snapshot.yaml
🚧 Files skipped from review as they are similar to previous changes (3)
  • pkg/variantregistry/snapshot.yaml
  • pkg/variantregistry/ocp_test.go
  • pkg/variantregistry/ocp.go

Walkthrough

Adds classification for -iso-no-registry jobs as candidate-tier installer variants with the NoRegistryClusterInstall capability, plus corresponding tests and snapshot entries.

Changes

No-registry installer classification

Layer / File(s) Summary
Classification and validation
pkg/variantregistry/ocp.go, pkg/variantregistry/ocp_test.go
Matches -iso-no-registry jobs as candidate-tier installer variants with the NoRegistryClusterInstall capability and validates parsed fields.
Snapshot synchronization
pkg/variantregistry/snapshot.yaml
Adds the no-registry capability and installer component to affected variant entries.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Suggested reviewers: petr-muller, smg247

🚥 Pre-merge checks | ✅ 21
✅ Passed checks (21 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: adding the NoRegistryClusterInstall capability to the variant registry.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Go Error Handling ✅ Passed The PR only adds a static variant matcher, test case, and snapshot entries; no new error paths, ignored errors, panics, or nil dereferences were introduced.
Sql Injection Prevention ✅ Passed The patch only adds variant matching/test data and snapshot entries; no SQL query construction was added or modified.
Excessive Css In React Should Use Styles ✅ Passed PASS: The PR only touches Go test/config files in pkg/variantregistry; no React components or inline CSS/style objects are present, so the CSS rule is not applicable.
Test Coverage For New Features ✅ Passed PASS: The new '-iso-no-registry' matcher is covered by a table-driven TestVariantSyncer case asserting component, capability, job tier, and related variants; snapshot updated too.
Single Responsibility And Clear Naming ✅ Passed The PR adds one narrowly-scoped matcher entry, a focused test, and snapshot updates; names are descriptive and the helper table stays small and cohesive.
Feature Documentation ✅ Passed No docs/features file covers variant-registry tracking; the only feature doc is unrelated, so a feature-doc update wasn’t applicable here.
Stable And Deterministic Test Names ✅ Passed The added test uses a fixed function name and a hardcoded job-string subtest title; no generated or time-varying identifiers appear.
Test Structure And Quality ✅ Passed The added test is a plain table-driven unit test (no Ginkgo/cluster ops), and its full-map assertion matches existing package patterns.
Microshift Test Compatibility ✅ Passed PASS: The added case is a plain table-driven unit test in ocp_test.go; it uses no Ginkgo/exutil and no unsupported OpenShift or MicroShift-only APIs/features.
Single Node Openshift (Sno) Test Compatibility ✅ Passed The added case is a plain table-driven unit test, not a Ginkgo e2e test, and it has no multi-node or SNO-sensitive assumptions.
Topology-Aware Scheduling Compatibility ✅ Passed Only variant-registry string/test/snapshot updates were changed; no deployment manifests, controllers, replicas, affinities, node selectors, or topology-aware scheduling logic.
Ote Binary Stdout Contract ✅ Passed Changed files only add variant mapping/test data; no fmt.Print/log/klog stdout writes or process-level setup appear in pkg/variantregistry.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed The added test is a Go unit test using testing/ testify, with no Ginkgo e2e constructs, IPv4 assumptions, or external connectivity.
No-Weak-Crypto ✅ Passed Changed files only add variant mapping/test data; no weak-crypto primitives, custom crypto, or secret comparisons were found.
Container-Privileges ✅ Passed No K8s/container manifest settings were added; the patch only updates variant-registry mappings/tests, and changed files contain no privileged/hostPID/hostIPC/allowPrivilegeEscalation fields.
No-Sensitive-Data-In-Logs ✅ Passed PR only adds a variant pattern and test/snapshot data; no new log statements or sensitive values were introduced.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@openshift-ci openshift-ci Bot requested review from petr-muller and smg247 July 14, 2026 15:05
@sadasu

sadasu commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

@dgoodwin , @JoelSpeed verify-feature-promotion results for the promotion of NoRegistryClusterInstall feature is unable to find their jobs.

Proposing this PR to fix that.

/cc @andfasano

@openshift-ci openshift-ci Bot added the ready-for-human-review Indicates a PR has been reviewed by automated tools and is ready for human review label Jul 14, 2026
@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Scheduling required tests:
/test e2e

@JoelSpeed

Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jul 14, 2026
@dgoodwin

Copy link
Copy Markdown
Contributor

Did this just happen due to #3731 merging?

I may have moved too fast there. My change removes the "install should succeed" hack, but it also now requires a full job pass, all tests. That is a major change for the installer. This is consistent with where I want this to go, I'd like jobs like this to map capability as you've done in your PR, but run a reduced and less sensitive set of tests (the main conformance suite is planned to be scaled down to this). These jobs full pass less than 30% of the time, you'll never make it through promotion. Working on what to do now, I think I need to revert my change for the time being. However these jobs can and should map their capability.

Comment thread pkg/variantregistry/ocp.go Outdated
{[]string{"-cpu-partitioning"}, "Node / Kubelet", "CPU Partitioning", "spotcheck-30d"},
{[]string{"-etcd-scaling"}, "Etcd", "Scaling", "spotcheck-30d"},
{[]string{"-aws-ovn-installer-dualstack"}, "Installer", "AWSDualStackInstall", "candidate"},
{[]string{"-iso-no-registry"}, "Installer", "NoRegistryClusterInstall", "candidate"},

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.

The component should match a jira component, which I believe is "Installer / openshift-installer"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The exact component type - that we also used to expose it in CR - it's Installer / Agent based installation

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Can we have a substring match?

@dgoodwin

Copy link
Copy Markdown
Contributor

/hold

@openshift-ci openshift-ci Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 14, 2026
@sadasu

sadasu commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

Did this just happen due to #3731 merging?

I may have moved too fast there. My change removes the "install should succeed" hack, but it also now requires a full job pass, all tests. That is a major change for the installer. This is consistent with where I want this to go, I'd like jobs like this to map capability as you've done in your PR, but run a reduced and less sensitive set of tests (the main conformance suite is planned to be scaled down to this). These jobs full pass less than 30% of the time, you'll never make it through promotion. Working on what to do now, I think I need to revert my change for the time being. However these jobs can and should map their capability.

Yes, that is a problem. The current problem I am trying to fix is regarding the discoverability of jobs with this featuregate. As part of openshift/api#2859, veirfy-feature-promotion gating test is unable to find the jobs in the 1st place.

This change adds Component and Capability tracking for Agent IRI No
registry nstaller jobs in the variant registry system.

Jobs matching the pattern '-iso-no-registry` are now classified with:

- Component: Installer
- Capability: NoRegistryClusterInstall
- JobTier: candidate

This enables Sippy to track these jobs separately via the capability-based
variant system introduced in PR openshift#3696.
@sadasu sadasu force-pushed the add-agent-iri-capability branch from 48528d7 to 984250c Compare July 14, 2026 17:38
@openshift-ci openshift-ci Bot removed the lgtm Indicates that a PR is ready to be merged. label Jul 14, 2026
@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Scheduling required tests:
/test e2e

@dgoodwin

Copy link
Copy Markdown
Contributor

/lgtm
/hold cancel

@openshift-ci openshift-ci Bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 14, 2026
@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jul 14, 2026
@openshift-ci

openshift-ci Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dgoodwin, JoelSpeed, sadasu

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 14, 2026
@openshift-ci

openshift-ci Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

@sadasu: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions 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. I understand the commands that are listed here.

@openshift-merge-bot openshift-merge-bot Bot merged commit f52a2bd into openshift:main Jul 14, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. ready-for-human-review Indicates a PR has been reviewed by automated tools and is ready for human review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants