Skip to content

Conversation

@gangwgr
Copy link
Contributor

@gangwgr gangwgr commented Nov 5, 2025

Refactor OTE to single-module architecture
As per discussion https://redhat-internal.slack.com/archives/CC3CZCQHM/p1762421042028719

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 5, 2025
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 5, 2025

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 5, 2025
@gangwgr gangwgr force-pushed the test-module-infra-only branch from 44e7612 to 70f4334 Compare November 5, 2025 07:26
@gangwgr gangwgr force-pushed the test-module-infra-only branch from 70f4334 to 615cb5f Compare November 5, 2025 07:33
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 5, 2025
@gangwgr gangwgr changed the title Create separate Go module for test extension CNTRLPLANE-1275:Create separate Go module for test extension Nov 5, 2025
@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Nov 5, 2025
@openshift-ci-robot
Copy link

openshift-ci-robot commented Nov 5, 2025

@gangwgr: This pull request references CNTRLPLANE-1275 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target either version "4.21." or "openshift-4.21.", but it targets "openshift-4.20" instead.

In response to this:

This commit isolates test dependencies from production code by creating a separate Go module for the test extension at test/extended/tests-extension/.

Key changes:

  • Move test files to test/extended/tests-extension/ directory structure
  • Create separate go.mod for test dependencies (ginkgo, gomega, openshift-tests-extension)
  • Remove test dependencies from root go.mod (~XXX lines removed from vendor)
  • Update root Makefile to use delegation pattern for test extension targets
  • Update Dockerfile.rhel7 paths to reference new binary location
  • Update .gitignore to ignore test/extended/tests-extension/bin/

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 openshift-eng/jira-lifecycle-plugin repository.

@gangwgr gangwgr marked this pull request as ready for review November 5, 2025 07:53
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 5, 2025
@openshift-ci openshift-ci bot requested review from atiratree and deads2k November 5, 2025 07:54
@gangwgr gangwgr force-pushed the test-module-infra-only branch from 615cb5f to db7055b Compare November 5, 2025 08:17
@gangwgr gangwgr changed the title CNTRLPLANE-1275:Create separate Go module for test extension [WIP]CNTRLPLANE-1275:Create separate Go module for test extension Nov 6, 2025
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 6, 2025
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 11, 2025
This commit isolates test dependencies from production code by creating a separate
Go module for the test extension at test/extended/tests-extension/.

Key changes:
- Move test files to test/extended/tests-extension/ directory structure
- Create separate go.mod for test dependencies (ginkgo, gomega, openshift-tests-extension)
- Remove test dependencies from root go.mod (~XXX lines removed from vendor)
- Update root Makefile to use delegation pattern for test extension targets
- Update Dockerfile.rhel7 paths to reference new binary location
- Update .gitignore to ignore test/extended/tests-extension/bin/

Benefits:
- Smaller production images (test dependencies not included in production builds)
- Faster builds (production builds don't need to vendor test dependencies)
- Cleaner dependency management (test dependencies isolated)
- Better CI performance (smaller images, faster pulls, less storage)

Related: Similar to openshift/openshift-apiserver#571
@gangwgr gangwgr force-pushed the test-module-infra-only branch from db7055b to 4f0d3b3 Compare November 24, 2025 09:00
@openshift-ci-robot
Copy link

openshift-ci-robot commented Nov 24, 2025

@gangwgr: This pull request references CNTRLPLANE-1275 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target either version "4.21." or "openshift-4.21.", but it targets "openshift-4.20" instead.

In response to this:

Refactor OTE to single-module architecture
As per discussion https://redhat-internal.slack.com/archives/CC3CZCQHM/p1762421042028719

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 openshift-eng/jira-lifecycle-plugin repository.

@gangwgr gangwgr changed the title [WIP]CNTRLPLANE-1275:Create separate Go module for test extension [WIP]CNTRLPLANE-1275:Refactor OTE to single-module architecture Nov 24, 2025
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 24, 2025

@gangwgr: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-tests-extension db7055b link true /test e2e-tests-extension
ci/prow/okd-scos-e2e-aws-ovn db7055b link false /test okd-scos-e2e-aws-ovn

Full PR test history. Your PR dashboard.

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-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 24, 2025
@gangwgr gangwgr force-pushed the test-module-infra-only branch 2 times, most recently from 9a9dce0 to 31d8124 Compare November 24, 2025 09:23
@gangwgr gangwgr changed the title [WIP]CNTRLPLANE-1275:Refactor OTE to single-module architecture CNTRLPLANE-1275:Refactor OTE to single-module architecture Nov 24, 2025
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 24, 2025
@gangwgr gangwgr force-pushed the test-module-infra-only branch from 31d8124 to 0c8e935 Compare November 24, 2025 10:19
@@ -0,0 +1,10 @@
reviewers:

Choose a reason for hiding this comment

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

Add QE team members.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added

.gitignore Outdated
_output
telepresence.log
.openshift-tests-extension/openshift_payload_*.json
/test/extended/tests-extension/bin/

Choose a reason for hiding this comment

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

I don't think we have this path.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed

@gangwgr gangwgr force-pushed the test-module-infra-only branch from 0c8e935 to 665b521 Compare November 24, 2025 10:30
- Updated library-go to v0.0.0-20251104164011-e9c2485b059c
- Resolved go.sum conflicts
- Re-vendored dependencies
@gangwgr gangwgr force-pushed the test-module-infra-only branch from 665b521 to 51f99cc Compare November 24, 2025 10:35
# See vendor/github.com/openshift/build-machinery-go/scripts/run-telepresence.sh for usage and configuration details
export TP_DEPLOYMENT_YAML ?=./manifests/0000_25_kube-controller-manager-operator_06_deployment.yaml
export TP_CMD_PATH ?=./cmd/cluster-kube-controller-manager-operator

Copy link
Member

Choose a reason for hiding this comment

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

Could we please add a new target called cluster-kube-controller-manager-operator to have the ability to simply build just the operator binary (e.g. for a development) withouth building the tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@atiratree Earlier we are have separate target, but Lukas suggested to use single target. We are following same standard in all control-plane repos

Copy link
Member

Choose a reason for hiding this comment

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

Yeah. that is fine, but having the ability to compile just the binary is also useful sometimes. Anyway, we can do that later I guess.

if v := version.Get().String(); len(v) == 0 {
cmd.Version = "<unknown>"
} else {
cmd.Version = v
Copy link
Member

Choose a reason for hiding this comment

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

who are the consumers of this version?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. The --version flag: Cobra automatically adds a --version flag (and short flag -v) to the root command when the Version field is set
  2. End users: Anyone running the cluster-kube-controller-manager-operator-tests-ext binary can use:
    ./cluster-kube-controller-manager-operator-tests-ext --version

Copy link
Member

Choose a reason for hiding this comment

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

Ack, so for now it is just for convenience.

@atiratree
Copy link
Member

/approve

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 26, 2025
@atiratree
Copy link
Member

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 26, 2025
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 26, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: atiratree, gangwgr

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

The pull request process is described here

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

@atiratree
Copy link
Member

build / ci passing
/verified by @atiratree

@openshift-ci-robot openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Nov 27, 2025
@openshift-ci-robot
Copy link

@atiratree: This PR has been marked as verified by @atiratree.

In response to this:

build / ci passing
/verified by @atiratree

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 openshift-eng/jira-lifecycle-plugin repository.

@openshift-merge-bot openshift-merge-bot bot merged commit 395de96 into openshift:main Nov 27, 2025
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. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. verified Signifies that the PR passed pre-merge verification criteria

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants