Skip to content

DNM: test aws ccm byo sg for NLBs#460

Draft
mfbonfigli wants to merge 3 commits intoopenshift:mainfrom
mfbonfigli:dnm/test-aws-ccm-byo-sg-nlb
Draft

DNM: test aws ccm byo sg for NLBs#460
mfbonfigli wants to merge 3 commits intoopenshift:mainfrom
mfbonfigli:dnm/test-aws-ccm-byo-sg-nlb

Conversation

@mfbonfigli
Copy link
Copy Markdown

@mfbonfigli mfbonfigli commented May 6, 2026

Do not merge

Empty PR just to exercise CI with upstream changes.

Summary by CodeRabbit

  • Chores
    • Updated Go toolchain to version 1.26.0
    • Upgraded AWS SDK and Kubernetes API library dependencies
    • Updated additional indirect package dependencies

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented May 6, 2026

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-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 May 6, 2026
@mfbonfigli
Copy link
Copy Markdown
Author

/testwith openshift/cloud-provider-aws/main/e2e-aws-ovn-techpreview openshift/cloud-provider-aws#152 openshift/installer#10512

@mtulio
Copy link
Copy Markdown
Contributor

mtulio commented May 6, 2026

@mfbonfigli you may need update ote go.mod to your version containing upstream e2e tests

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 7, 2026

Warning

Rate limit exceeded

@mfbonfigli has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 23 minutes and 7 seconds before requesting another review.

To continue reviewing without waiting, purchase usage credits in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 157f0ed7-3a62-4795-b8ee-9332a9f37303

📥 Commits

Reviewing files that changed from the base of the PR and between 82df713 and d00d962.

⛔ Files ignored due to path filters (2)
  • vendor/k8s.io/cloud-provider-aws/tests/e2e/loadbalancer.go is excluded by !**/vendor/**, !vendor/**
  • vendor/modules.txt is excluded by !**/vendor/**, !vendor/**
📒 Files selected for processing (1)
  • openshift-tests/ccm-aws-tests/go.mod

Walkthrough

This PR updates the Go toolchain from 1.25.0 to 1.26.0 in go.mod and upgrades primary and indirect dependencies, including AWS SDK modules and Kubernetes libraries to v0.36.0. A new replace directive for cloud-provider-aws/tests/e2e is added.

Changes

Dependency & Toolchain Updates

Layer / File(s) Summary
Go Toolchain Version
openshift-tests/ccm-aws-tests/go.mod
Go language version bumped from 1.25.0 to 1.26.0.
Direct Dependencies
openshift-tests/ccm-aws-tests/go.mod
AWS SDK modules, Kubernetes API libraries (k8s.io/api, k8s.io/apimachinery, k8s.io/client-go), and related test/client dependencies upgraded to v0.36.0.
Indirect Dependencies
openshift-tests/ccm-aws-tests/go.mod
AWS SDK internal packages, go.opentelemetry.io/*, golang.org/x/*, google.golang.org/*, and Kubernetes-related transitive modules updated across the dependency graph.
Module Replacement Directives
openshift-tests/ccm-aws-tests/go.mod
New replace rule added for k8s.io/cloud-provider-aws/tests/e2e pointing to a newer commit of github.com/mfbonfigli/cloud-provider-aws/tests/e2e; existing replacements for onsi-ginkgo/v2 and Kubernetes modules retained.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error, 1 warning)

Check name Status Explanation Resolution
Stable And Deterministic Test Names ❌ Error The Describe block at line 47 uses fmt.Sprintf() to construct the test name, violating the requirement for static test titles. Replace fmt.Sprintf() with a static string literal: "[cloud-provider-aws-e2e-openshift] loadbalancer NLB [OCPFeatureGate:AWSServiceLBNetworkSecurityGroup]"
Title check ⚠️ Warning The PR title 'DNM: test aws ccm byo sg for NLBs' refers to testing AWS CCM with custom security groups for NLBs, but the actual changeset only updates Go toolchain version and dependencies in go.mod without implementing any test functionality. Update the title to reflect the actual change, such as 'Update go.mod for AWS CCM e2e tests' or 'Upgrade Go and dependencies for CCM e2e testing' to accurately represent the dependency update.
✅ Passed checks (10 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
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.
Test Structure And Quality ✅ Passed All test quality requirements met: single responsibility per test, proper setup/cleanup with DeferCleanup, appropriate timeouts, meaningful assertion messages, framework consistency.
Microshift Test Compatibility ✅ Passed New tests in loadbalancer.go have [OCPFeatureGate:AWSServiceLBNetworkSecurityGroup] tag. Only standard Kubernetes and AWS APIs used. No unavailable MicroShift APIs referenced.
Single Node Openshift (Sno) Test Compatibility ✅ Passed PR #460 only modifies go.mod with dependency updates. No new Ginkgo e2e tests (It/Describe/Context) are added. The SNO compatibility check applies only to new tests, so it's not applicable here.
Topology-Aware Scheduling Compatibility ✅ Passed PR only updates go.mod dependencies for e2e tests. No deployment manifests, operator code, or controllers are modified. No scheduling constraints are introduced. Check not applicable.
Ote Binary Stdout Contract ✅ Passed PR updates only go.mod/go.sum with zero code changes. No stdout-writing patterns found in codebase. Existing code complies with OTE stdout contract.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed The new Ginkgo e2e tests in loadbalancer.go contain no IPv4-only assumptions or external connectivity requirements. All networking is to AWS APIs or local cluster services.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with 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.

❤️ Share

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

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented May 7, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign mdbooth for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found 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

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
openshift-tests/ccm-aws-tests/go.mod (1)

3-3: Consider updating to Go 1.26.2, the latest stable release.

Go 1.26.0 is a valid released version. However, go1.26.2 (released 2026-04-07) is now available and is the current stable version. Updating to the latest patch release would ensure access to recent bug fixes and improvements.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@openshift-tests/ccm-aws-tests/go.mod` at line 3, Update the Go toolchain
version in the go.mod file by changing the module directive value from "go
1.26.0" to "go 1.26.2": open the go.mod and replace the existing go version
token (the "go" directive) so the project uses Go 1.26.2, then run `go mod tidy`
or your project's dependency bootstrap to ensure tooling picks up the updated
version.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@openshift-tests/ccm-aws-tests/go.mod`:
- Around line 16-18: The go.mod currently mixes Kubernetes module versions
(k8s.io/api, k8s.io/apimachinery, k8s.io/client-go at v0.36.0 while
k8s.io/kubernetes remains v1.35.0 and several components are replaced/pinned to
v0.35.0 via replace directives), which can cause resolution and API mismatches;
either confirm this skew is intentional for your tests or make the versions
consistent: to align downward, change k8s.io/api, k8s.io/apimachinery, and
k8s.io/client-go to v0.35.0 to match the replace pins and k8s.io/kubernetes; or
to align upward, bump k8s.io/kubernetes and all replace directives to
v1.36.0/v0.36.0 as appropriate and run go mod tidy to refresh module graph and
verify e2e tests; update any replace directives referencing Kubernetes
components to the chosen consistent version.

---

Nitpick comments:
In `@openshift-tests/ccm-aws-tests/go.mod`:
- Line 3: Update the Go toolchain version in the go.mod file by changing the
module directive value from "go 1.26.0" to "go 1.26.2": open the go.mod and
replace the existing go version token (the "go" directive) so the project uses
Go 1.26.2, then run `go mod tidy` or your project's dependency bootstrap to
ensure tooling picks up the updated version.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: f5354a3a-29f9-42b8-8e83-8f0e89f77f08

📥 Commits

Reviewing files that changed from the base of the PR and between ff92368 and 82df713.

⛔ Files ignored due to path filters (1)
  • openshift-tests/ccm-aws-tests/go.sum is excluded by !**/*.sum
📒 Files selected for processing (1)
  • openshift-tests/ccm-aws-tests/go.mod

Comment thread openshift-tests/ccm-aws-tests/go.mod Outdated
@mfbonfigli
Copy link
Copy Markdown
Author

/testwith openshift/cloud-provider-aws/main/e2e-aws-ovn-techpreview openshift/cloud-provider-aws#152 openshift/installer#10512

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants