DNM/SPLAT-2672: e2e/ote/ccm-aws: moving binary cccmo project#437
Draft
mtulio wants to merge 2 commits intoopenshift:mainfrom
Draft
DNM/SPLAT-2672: e2e/ote/ccm-aws: moving binary cccmo project#437mtulio wants to merge 2 commits intoopenshift:mainfrom
mtulio wants to merge 2 commits intoopenshift:mainfrom
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@mtulio: This pull request references SPLAT-2672 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 task to target the "4.22.0" version, but no target version was set. DetailsIn response to this: 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-ci
bot
added
the
do-not-merge/work-in-progress
Contributor
|
Skipping CI for Draft Pull Request. |
WalkthroughAdds an AWS CCM tests extension: new test runner binary, module and deps, Docker/Make build updates, README, AWS/OpenShift e2e helpers, and a Network Load Balancer security-group focused e2e test suite. Changes
Sequence Diagram(s)sequenceDiagram
participant Runner as Test Runner
participant Framework as Test Framework
participant K8s as Kubernetes API
participant AWS as AWS SDK
participant Cloud as Cloud Config
Runner->>Framework: Initialize (KUBECONFIG, region)
Runner->>Framework: Register & filter AWS CCM specs
Runner->>Framework: Execute suite
Note over Framework: NLB security-group test flow
Framework->>Cloud: Check AWSServiceLBNetworkSecurityGroup FeatureGate
Framework->>Cloud: Verify NLBSecurityGroupMode in cloud-config
Framework->>K8s: Create Service(type=NLB)
K8s-->>Framework: Service provisioned (LB DNS)
Framework->>AWS: DescribeLoadBalancers(by DNS)
AWS-->>Framework: LoadBalancer metadata (SG IDs)
Framework->>AWS: DescribeSecurityGroups(SG IDs)
AWS-->>Framework: Security group rules
Framework->>Framework: Validate SG rules vs expected ports
Framework->>K8s: Delete Service
Framework->>AWS: Poll for LoadBalancer deletion
AWS-->>Framework: Confirm LB removed
Framework-->>Runner: Report test result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
|
@mtulio: This pull request references SPLAT-2672 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 task to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
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. |
Contributor
Author
|
/testwith openshift/cluster-cloud-controller-manager-operator/main/e2e-aws openshift/origin/#30864 |
Contributor
|
@mtulio, |
Contributor
Author
|
/testwith openshift/cluster-cloud-controller-manager-operator/main/e2e-aws openshift/origin#30864 |
Contributor
|
@mtulio, |
Contributor
Author
|
/test ? |
Contributor
Author
|
/testwith openshift/cluster-cloud-controller-manager-operator/main/pull-ci-openshift-cloud-provider-aws-main-e2e-aws-ovn openshift/origin#30864 |
Contributor
|
@mtulio, |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (5)
Makefile (2)
40-40: Consider declaringbuildas.PHONY.The static analysis tool flagged that the
buildtarget should be declared as PHONY since it doesn't produce a file named "build". This ensures the target always runs regardless of file system state.♻️ Proposed fix
+.PHONY: build build: operator config-sync-controllers azure-config-credentials-injector cloud-controller-manager-aws-tests-ext🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` at line 40, The Makefile's build target ("build: operator config-sync-controllers azure-config-credentials-injector cloud-controller-manager-aws-tests-ext") should be declared phony so it always runs; add a .PHONY declaration that includes build (e.g., add ".PHONY: build" alongside any other phony targets) so the make tool doesn't treat "build" as a real file and will always execute the recipe.
51-56: Missingpopdafterpushdin shell command.The
pushdcommand changes the directory, but there's no correspondingpopdto restore the original directory. While Make targets typically run in subshells (so the effect is limited), it's good practice to pairpushdwithpopdfor clarity and to avoid issues if the command is expanded later.♻️ Proposed fix
cloud-controller-manager-aws-tests-ext: pushd cmd/cloud-controller-manager-aws-tests-ext && \ GO111MODULE=on CGO_ENABLED=0 GOOS=$(GOOS) GOARCH=$(GOARCH) GOPROXY=$(GOPROXY) go build \ -trimpath \ -ldflags="$(LDFLAGS)" \ - -o=../../bin/cloud-controller-manager-aws-tests-ext . - + -o=../../bin/cloud-controller-manager-aws-tests-ext . && \ + popd🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 51 - 56, The target cloud-controller-manager-aws-tests-ext uses pushd to change directories but never restores the original directory; update the shell recipe so pushd cmd/cloud-controller-manager-aws-tests-ext is paired with a corresponding popd after the go build step (e.g., ensure the sequence ends with popd or chain commands so popd always runs even on failure) to restore the working directory.cmd/cloud-controller-manager-aws-tests-ext/main.go (2)
173-177: Typo in comment."leding" should be "leading".
📝 Proposed fix
// After reading all flags, this will configure the test context, and need to be // called once by framework to avoid re-configuring the test context, and leding - // to issues in Ginkgo phases (PhaseBuildTopLevel, PhaseBuildTree, PhaseRun), + // leading to issues in Ginkgo phases (PhaseBuildTopLevel, PhaseBuildTree, PhaseRun), // such as:'cannot clone suite after tree has been built'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/cloud-controller-manager-aws-tests-ext/main.go` around lines 173 - 177, Fix the typo in the comment above the framework.AfterReadingAllFlags(testContext) call: change "leding" to "leading" so the comment reads "...and leading to issues in Ginkgo phases (PhaseBuildTopLevel, PhaseBuildTree, PhaseRun), such as:'cannot clone suite after tree has been built'".
105-124: Getter function has unexpected side effects.
getRegionFromEnvsetsAWS_REGIONas a side effect when reading fromLEASED_RESOURCEorAWS_DEFAULT_REGION. This violates the principle of least surprise for a "get" function. Consider renaming togetAndSetRegionFromEnvor extracting the side effect into a separate function.♻️ Proposed refactor
-// getRegionFromEnv gets the region from the environment variables. -func getRegionFromEnv() string { +// getOrSetRegionFromEnv gets the region from environment variables and ensures +// AWS_REGION is set for downstream AWS SDK usage. +func getOrSetRegionFromEnv() string { region := os.Getenv("LEASED_RESOURCE") if len(region) > 0 { log.Debugf("Using region from LEASED_RESOURCE: %s", region)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/cloud-controller-manager-aws-tests-ext/main.go` around lines 105 - 124, The function getRegionFromEnv currently mutates environment variables (calls os.Setenv for AWS_REGION) when reading LEASED_RESOURCE or AWS_DEFAULT_REGION; either remove these side effects so the function only returns the resolved region, or split into two functions: getRegionFromEnv (pure getter that reads LEASED_RESOURCE, AWS_REGION, AWS_DEFAULT_REGION and returns the region) and setRegionEnv(region string) which performs os.Setenv("AWS_REGION", region) when desired; if you prefer keeping behavior, rename getRegionFromEnv to getAndSetRegionFromEnv to make the side effect explicit and update all call sites accordingly.cmd/cloud-controller-manager-aws-tests-ext/e2e/helper.go (1)
187-196: Use AWS SDK error types instead of string matching.The
ec2IsNotFoundErrorfunction relies on string matching against error messages, which is fragile. AWS SDK v2 provides typed error handling viasmithyerrors. Additionally,InvalidGroup.Malformedindicates an invalid ID format, not a non-existent group—it should likely cause test failure rather than being treated as "not found."♻️ Proposed fix using AWS SDK error types
+import ( + "errors" + "github.com/aws/smithy-go" +) + // ec2IsNotFoundError checks if an error is an EC2 "not found" error. func ec2IsNotFoundError(err error) bool { if err == nil { return false } - // Check for common EC2 not found error messages - errMsg := err.Error() - return strings.Contains(errMsg, "InvalidGroup.NotFound") || - strings.Contains(errMsg, "InvalidGroupId.NotFound") || - strings.Contains(errMsg, "InvalidGroup.Malformed") + var apiErr smithy.APIError + if errors.As(err, &apiErr) { + switch apiErr.ErrorCode() { + case "InvalidGroup.NotFound", "InvalidGroupId.NotFound": + return true + } + } + return false }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/cloud-controller-manager-aws-tests-ext/e2e/helper.go` around lines 187 - 196, Replace fragile string matching in ec2IsNotFoundError with typed AWS SDK error handling: use errors.As to check if err implements github.com/aws/smithy-go/middleware/APIError (smithy.APIError) or smithy.APIError interface and then switch on apiErr.ErrorCode() to return true only for "InvalidGroup.NotFound" and "InvalidGroupId.NotFound"; explicitly do NOT treat "InvalidGroup.Malformed" as not-found (return false) so malformed IDs cause test failures; update ec2IsNotFoundError to use errors.As and ErrorCode() comparisons instead of strings.Contains.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/cloud-controller-manager-aws-tests-ext/e2e/loadbalancer.go`:
- Around line 296-304: The call to framework.ExpectNoError inside the Eventually
callback prevents retries on transient errors; update the callback so
getAWSSecurityGroupRules(ctx, ec2Client, foundLB.SecurityGroups) returns its
error instead of calling framework.ExpectNoError. Specifically, replace the
ExpectNoError call with logic that checks err and returns nil, err (or
fmt.Errorf-wrapped err) so Eventually can retry; keep the subsequent length
check using originalSGRules and currentSGRules and return the existing
fmt.Errorf when counts haven't changed.
- Around line 292-294: The log message in the load balancer check (inside the
block that inspects foundLB and foundLB.SecurityGroups) is misleading because it
uses "expected=%d" while printing the actual count len(foundLB.SecurityGroups);
update the framework.Logf call to clearly state actual vs expected (for example
"Load balancer has no security groups attached: actual=%d expected=1" or simply
"Load balancer has no security groups attached: actual=%d"), using
len(foundLB.SecurityGroups) for the actual value, and leave the error return
(fmt.Errorf("load balancer has no security groups attached")) unchanged.
In `@cmd/cloud-controller-manager-aws-tests-ext/go.mod`:
- Line 132: The go.mod currently pulls k8s.io/cloud-provider v0.34.0 indirectly
which conflicts with other Kubernetes modules pinned at v0.35.0; update the
module replacement by adding a replace directive for k8s.io/cloud-provider to
v0.35.0 (i.e., add "replace k8s.io/cloud-provider => k8s.io/cloud-provider
v0.35.0" in go.mod) so the dependency graph resolves to the same Kubernetes
version, or alternatively add a brief comment in go.mod explaining why v0.34.0
must remain if there is a compatibility reason.
In `@cmd/cloud-controller-manager-aws-tests-ext/main.go`:
- Around line 191-193: The condition currently uses
len(os.Getenv("TEST_JUNIT_DIR")) >= 0 which is always true; change the check to
only set testContext.ReportDir when the environment variable is non-empty (e.g.,
testDir := os.Getenv("TEST_JUNIT_DIR"); if testDir != "" { testContext.ReportDir
= testDir }) so that testContext.ReportDir is not overwritten with an empty
string; update references to os.Getenv("TEST_JUNIT_DIR") and
testContext.ReportDir accordingly.
In `@cmd/cloud-controller-manager-aws-tests-ext/README.md`:
- Around line 46-50: Update the README to use the correct make target and output
path: replace the incorrect make invocation that references
openshift-hack/Makefile with the root Makefile target "make
cloud-controller-manager-aws-tests-ext", and update the described binary
location from "./aws-cloud-controller-manager-tests-ext" to
"./bin/cloud-controller-manager-aws-tests-ext" so the command and output path
match the actual "cloud-controller-manager-aws-tests-ext" build target.
- Line 39: Update the README entry that currently says "Go 1.24+" to match the
module's declared Go version in go.mod (go 1.25.0); edit the README text so the
prerequisite reflects "Go 1.25.0" or "Go 1.25+" to align with the go directive
in go.mod and avoid version mismatch.
---
Nitpick comments:
In `@cmd/cloud-controller-manager-aws-tests-ext/e2e/helper.go`:
- Around line 187-196: Replace fragile string matching in ec2IsNotFoundError
with typed AWS SDK error handling: use errors.As to check if err implements
github.com/aws/smithy-go/middleware/APIError (smithy.APIError) or
smithy.APIError interface and then switch on apiErr.ErrorCode() to return true
only for "InvalidGroup.NotFound" and "InvalidGroupId.NotFound"; explicitly do
NOT treat "InvalidGroup.Malformed" as not-found (return false) so malformed IDs
cause test failures; update ec2IsNotFoundError to use errors.As and ErrorCode()
comparisons instead of strings.Contains.
In `@cmd/cloud-controller-manager-aws-tests-ext/main.go`:
- Around line 173-177: Fix the typo in the comment above the
framework.AfterReadingAllFlags(testContext) call: change "leding" to "leading"
so the comment reads "...and leading to issues in Ginkgo phases
(PhaseBuildTopLevel, PhaseBuildTree, PhaseRun), such as:'cannot clone suite
after tree has been built'".
- Around line 105-124: The function getRegionFromEnv currently mutates
environment variables (calls os.Setenv for AWS_REGION) when reading
LEASED_RESOURCE or AWS_DEFAULT_REGION; either remove these side effects so the
function only returns the resolved region, or split into two functions:
getRegionFromEnv (pure getter that reads LEASED_RESOURCE, AWS_REGION,
AWS_DEFAULT_REGION and returns the region) and setRegionEnv(region string) which
performs os.Setenv("AWS_REGION", region) when desired; if you prefer keeping
behavior, rename getRegionFromEnv to getAndSetRegionFromEnv to make the side
effect explicit and update all call sites accordingly.
In `@Makefile`:
- Line 40: The Makefile's build target ("build: operator config-sync-controllers
azure-config-credentials-injector cloud-controller-manager-aws-tests-ext")
should be declared phony so it always runs; add a .PHONY declaration that
includes build (e.g., add ".PHONY: build" alongside any other phony targets) so
the make tool doesn't treat "build" as a real file and will always execute the
recipe.
- Around line 51-56: The target cloud-controller-manager-aws-tests-ext uses
pushd to change directories but never restores the original directory; update
the shell recipe so pushd cmd/cloud-controller-manager-aws-tests-ext is paired
with a corresponding popd after the go build step (e.g., ensure the sequence
ends with popd or chain commands so popd always runs even on failure) to restore
the working directory.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 202486ca-22bf-47b5-9126-7a3de9893062
⛔ Files ignored due to path filters (292)
cmd/cloud-controller-manager-aws-tests-ext/go.sumis excluded by!**/*.sumcmd/cloud-controller-manager-aws-tests-ext/vendor/cel.dev/expr/.bazelversionis excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/cel.dev/expr/.gitattributesis excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/cel.dev/expr/.gitignoreis excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/cel.dev/expr/BUILD.bazelis excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/cel.dev/expr/CODE_OF_CONDUCT.mdis excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/cel.dev/expr/CONTRIBUTING.mdis excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/cel.dev/expr/GOVERNANCE.mdis excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/cel.dev/expr/LICENSEis excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/cel.dev/expr/MAINTAINERS.mdis excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/cel.dev/expr/MODULE.bazelis excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/cel.dev/expr/README.mdis excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/cel.dev/expr/WORKSPACEis excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/cel.dev/expr/WORKSPACE.bzlmodis excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/cel.dev/expr/checked.pb.gois excluded by!**/*.pb.go,!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/cel.dev/expr/cloudbuild.yamlis excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/cel.dev/expr/eval.pb.gois excluded by!**/*.pb.go,!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/cel.dev/expr/explain.pb.gois excluded by!**/*.pb.go,!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/cel.dev/expr/regen_go_proto.shis excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/cel.dev/expr/regen_go_proto_canonical_protos.shis excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/cel.dev/expr/syntax.pb.gois excluded by!**/*.pb.go,!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/cel.dev/expr/value.pb.gois excluded by!**/*.pb.go,!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/Masterminds/semver/v3/.gitignoreis excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/Masterminds/semver/v3/.golangci.ymlis excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/Masterminds/semver/v3/CHANGELOG.mdis excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/Masterminds/semver/v3/LICENSE.txtis excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/Masterminds/semver/v3/Makefileis excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/Masterminds/semver/v3/README.mdis excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/Masterminds/semver/v3/SECURITY.mdis excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/Masterminds/semver/v3/collection.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/Masterminds/semver/v3/constraints.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/Masterminds/semver/v3/doc.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/Masterminds/semver/v3/version.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/.gitignoreis excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/LICENSEis excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/README.mdis excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/antlrdoc.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/atn.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/atn_config.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/atn_config_set.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/atn_deserialization_options.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/atn_deserializer.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/atn_simulator.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/atn_state.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/atn_type.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/char_stream.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/common_token_factory.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/common_token_stream.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/comparators.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/configuration.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/dfa.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/dfa_serializer.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/dfa_state.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/diagnostic_error_listener.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/error_listener.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/error_strategy.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/errors.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/file_stream.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/input_stream.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/int_stream.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/interval_set.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/jcollect.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/lexer.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/lexer_action.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/lexer_action_executor.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/lexer_atn_simulator.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/ll1_analyzer.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/nostatistics.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/parser.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/parser_atn_simulator.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/parser_rule_context.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/prediction_context.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/prediction_context_cache.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/prediction_mode.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/recognizer.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/rule_context.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/semantic_context.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/statistics.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/stats_data.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/token.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/token_source.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/token_stream.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/tokenstream_rewriter.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/trace_listener.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/transition.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/tree.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/trees.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/antlr4-go/antlr/v4/utils.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/LICENSE.txtis excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/NOTICE.txtis excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/accountid_endpoint_mode.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/checksum.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/config.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/context.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/credential_cache.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/credentials.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/defaults/auto.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/defaults/configuration.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/defaults/defaults.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/defaults/doc.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/defaultsmode.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/doc.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/endpoints.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/errors.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/from_ptr.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/go_module_metadata.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/logging.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/logging_generate.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/metadata.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/middleware.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/osname.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/osname_go115.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/recursion_detection.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/request_id.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/request_id_retriever.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/user_agent.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/ec2query/error_utils.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/query/array.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/query/encoder.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/query/map.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/query/middleware.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/query/object.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/query/value.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/restjson/decoder_util.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/xml/error_utils.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/ratelimit/none.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/ratelimit/token_bucket.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/ratelimit/token_rate_limit.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/request.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/retry/adaptive.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/retry/adaptive_ratelimit.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/retry/adaptive_token_bucket.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/retry/attempt_metrics.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/retry/doc.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/retry/errors.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/retry/jitter_backoff.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/retry/metadata.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/retry/middleware.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/retry/retry.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/retry/retryable_error.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/retry/standard.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/retry/throttle_error.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/retry/timeout_error.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/retryer.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/runtime.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4/cache.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4/const.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4/header_rules.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4/headers.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4/hmac.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4/host.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4/scope.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4/time.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4/util.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/signer/v4/middleware.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/signer/v4/presign_middleware.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/signer/v4/stream.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/signer/v4/v4.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/to_ptr.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/transport/http/client.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/transport/http/content_type.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/transport/http/response_error.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/transport/http/response_error_middleware.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/transport/http/timeout_read_closer.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/types.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/aws/version.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/config/CHANGELOG.mdis excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/config/LICENSE.txtis excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/config/config.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/config/defaultsmode.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/config/doc.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/config/env_config.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/config/generate.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/config/go_module_metadata.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/config/load_options.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/config/local.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/config/provider.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/config/resolve.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/config/resolve_bearer_token.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/config/resolve_credentials.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/config/shared_config.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/credentials/CHANGELOG.mdis excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/credentials/LICENSE.txtis excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/credentials/doc.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/credentials/ec2rolecreds/doc.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/credentials/ec2rolecreds/provider.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/credentials/endpointcreds/internal/client/auth.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/credentials/endpointcreds/internal/client/client.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/credentials/endpointcreds/internal/client/endpoints.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/credentials/endpointcreds/internal/client/middleware.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/credentials/endpointcreds/provider.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/credentials/go_module_metadata.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/credentials/processcreds/doc.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/credentials/processcreds/provider.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/credentials/ssocreds/doc.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/credentials/ssocreds/sso_cached_token.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/credentials/ssocreds/sso_credentials_provider.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/credentials/ssocreds/sso_token_provider.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/credentials/static_provider.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/credentials/stscreds/assume_role_provider.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/credentials/stscreds/web_identity_provider.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/CHANGELOG.mdis excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/LICENSE.txtis excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/api_client.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/api_op_GetDynamicData.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/api_op_GetIAMInfo.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/api_op_GetInstanceIdentityDocument.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/api_op_GetMetadata.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/api_op_GetRegion.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/api_op_GetToken.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/api_op_GetUserData.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/auth.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/doc.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/endpoints.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/go_module_metadata.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/internal/config/resolvers.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/request_middleware.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/token_provider.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/internal/auth/auth.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/internal/auth/scheme.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/internal/auth/smithy/bearer_token_adapter.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/internal/auth/smithy/bearer_token_signer_adapter.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/internal/auth/smithy/credentials_adapter.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/internal/auth/smithy/smithy.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/internal/auth/smithy/v4signer_adapter.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/internal/configsources/CHANGELOG.mdis excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/internal/configsources/LICENSE.txtis excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/internal/configsources/config.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/internal/configsources/endpoints.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/internal/configsources/go_module_metadata.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/internal/context/context.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/awsrulesfn/arn.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/awsrulesfn/doc.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/awsrulesfn/generate.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/awsrulesfn/host.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/awsrulesfn/partition.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/awsrulesfn/partitions.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/awsrulesfn/partitions.jsonis excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/endpoints.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/v2/CHANGELOG.mdis excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/v2/LICENSE.txtis excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/v2/endpoints.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/v2/go_module_metadata.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/internal/ini/CHANGELOG.mdis excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/internal/ini/LICENSE.txtis excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/internal/ini/errors.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/internal/ini/go_module_metadata.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/internal/ini/ini.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/internal/ini/parse.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/internal/ini/sections.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/internal/ini/strings.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/internal/ini/token.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/internal/ini/tokenize.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/internal/ini/value.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/internal/middleware/middleware.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/internal/rand/rand.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/internal/sdk/interfaces.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/internal/sdk/time.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/internal/sdkio/byte.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/internal/shareddefaults/shared_config.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/internal/strings/strings.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/internal/sync/singleflight/LICENSEis excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/internal/sync/singleflight/docs.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/internal/sync/singleflight/singleflight.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/internal/timeconv/duration.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/service/ec2/CHANGELOG.mdis excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/service/ec2/LICENSE.txtis excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_client.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AcceptAddressTransfer.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AcceptCapacityReservationBillingOwnership.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AcceptReservedInstancesExchangeQuote.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AcceptTransitGatewayMulticastDomainAssociations.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AcceptTransitGatewayPeeringAttachment.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AcceptTransitGatewayVpcAttachment.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AcceptVpcEndpointConnections.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AcceptVpcPeeringConnection.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AdvertiseByoipCidr.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AllocateAddress.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AllocateHosts.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AllocateIpamPoolCidr.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_ApplySecurityGroupsToClientVpnTargetNetwork.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssignIpv6Addresses.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssignPrivateIpAddresses.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssignPrivateNatGatewayAddress.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssociateAddress.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssociateCapacityReservationBillingOwner.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssociateClientVpnTargetNetwork.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssociateDhcpOptions.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssociateEnclaveCertificateIamRole.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssociateIamInstanceProfile.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssociateInstanceEventWindow.gois excluded by!**/vendor/**cmd/cloud-controller-manager-aws-tests-ext/vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssociateIpamByoasn.gois excluded by!**/vendor/**
📒 Files selected for processing (8)
.dockerignoreDockerfileMakefilecmd/cloud-controller-manager-aws-tests-ext/README.mdcmd/cloud-controller-manager-aws-tests-ext/e2e/helper.gocmd/cloud-controller-manager-aws-tests-ext/e2e/loadbalancer.gocmd/cloud-controller-manager-aws-tests-ext/go.modcmd/cloud-controller-manager-aws-tests-ext/main.go
Comment on lines
+296
to
+304
|
|
||
| currentSGRules, err := getAWSSecurityGroupRules(ctx, ec2Client, foundLB.SecurityGroups) | ||
| framework.ExpectNoError(err, "failed to get security group rules to calculate the diff") | ||
|
|
||
| if len(originalSGRules) >= len(currentSGRules) { | ||
| framework.Logf("Security group rules count did not changed: original=%d current=%d", | ||
| len(originalSGRules), len(currentSGRules)) | ||
| return nil, fmt.Errorf("security group rules count did not changed") | ||
| } |
There was a problem hiding this comment.
Avoid framework.ExpectNoError inside Eventually callback.
Using framework.ExpectNoError inside the Eventually callback will cause an immediate Ginkgo assertion failure on transient errors, preventing Eventually from retrying. Return the error instead to allow proper retry behavior.
🐛 Proposed fix
currentSGRules, err := getAWSSecurityGroupRules(ctx, ec2Client, foundLB.SecurityGroups)
- framework.ExpectNoError(err, "failed to get security group rules to calculate the diff")
+ if err != nil {
+ framework.Logf("failed to get security group rules: %v", err)
+ return nil, err
+ }
if len(originalSGRules) >= len(currentSGRules) {
framework.Logf("Security group rules count did not changed: original=%d current=%d",
len(originalSGRules), len(currentSGRules))
- return nil, fmt.Errorf("security group rules count did not changed")
+ return nil, fmt.Errorf("security group rules count did not change")
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/cloud-controller-manager-aws-tests-ext/e2e/loadbalancer.go` around lines
296 - 304, The call to framework.ExpectNoError inside the Eventually callback
prevents retries on transient errors; update the callback so
getAWSSecurityGroupRules(ctx, ec2Client, foundLB.SecurityGroups) returns its
error instead of calling framework.ExpectNoError. Specifically, replace the
ExpectNoError call with logic that checks err and returns nil, err (or
fmt.Errorf-wrapped err) so Eventually can retry; keep the subsequent length
check using originalSGRules and currentSGRules and return the existing
fmt.Errorf when counts haven't changed.
| gopkg.in/yaml.v3 v3.0.1 // indirect | ||
| k8s.io/apiextensions-apiserver v0.0.0 // indirect | ||
| k8s.io/apiserver v0.35.0 // indirect | ||
| k8s.io/cloud-provider v0.34.0 // indirect |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if cloud-provider v0.34.0 is intentional due to transitive dependencies
cd cmd/cloud-controller-manager-aws-tests-ext
go mod graph | grep cloud-provider | head -20Repository: openshift/cluster-cloud-controller-manager-operator
Length of output: 1533
🏁 Script executed:
cat cmd/cloud-controller-manager-aws-tests-ext/go.mod | grep -E 'k8s\.io' | head -30Repository: openshift/cluster-cloud-controller-manager-operator
Length of output: 1577
🏁 Script executed:
# Check if there are any comments or context around this dependency
sed -n '125,140p' cmd/cloud-controller-manager-aws-tests-ext/go.modRepository: openshift/cluster-cloud-controller-manager-operator
Length of output: 817
Add a replace directive for k8s.io/cloud-provider to align with other Kubernetes module versions.
The indirect dependency k8s.io/cloud-provider v0.34.0 is mismatched with other Kubernetes modules (all at v0.35.0 via direct dependencies and replace directives). This causes the build to resolve multiple versions of the same module (e.g., k8s.io/api v0.35.0 and k8s.io/api v0.34.0 via transitive dependencies), which can lead to API incompatibilities. Add a replace directive:
k8s.io/cloud-provider => k8s.io/cloud-provider v0.35.0
Or document why v0.34.0 is intentional if there is a compatibility reason for the version mismatch.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/cloud-controller-manager-aws-tests-ext/go.mod` at line 132, The go.mod
currently pulls k8s.io/cloud-provider v0.34.0 indirectly which conflicts with
other Kubernetes modules pinned at v0.35.0; update the module replacement by
adding a replace directive for k8s.io/cloud-provider to v0.35.0 (i.e., add
"replace k8s.io/cloud-provider => k8s.io/cloud-provider v0.35.0" in go.mod) so
the dependency graph resolves to the same Kubernetes version, or alternatively
add a brief comment in go.mod explaining why v0.34.0 must remain if there is a
compatibility reason.
Comment on lines
+191
to
+193
| if len(os.Getenv("TEST_JUNIT_DIR")) >= 0 { | ||
| testContext.ReportDir = os.Getenv("TEST_JUNIT_DIR") | ||
| } |
There was a problem hiding this comment.
Condition is always true - likely a bug.
The condition len(os.Getenv("TEST_JUNIT_DIR")) >= 0 is always true because string length is never negative. This means testContext.ReportDir will always be set, even when TEST_JUNIT_DIR is empty (resulting in an empty ReportDir).
🐛 Proposed fix
// Output the logs on failure
- if len(os.Getenv("TEST_JUNIT_DIR")) >= 0 {
+ if len(os.Getenv("TEST_JUNIT_DIR")) > 0 {
testContext.ReportDir = os.Getenv("TEST_JUNIT_DIR")
}📝 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.
Suggested change
| if len(os.Getenv("TEST_JUNIT_DIR")) >= 0 { | |
| testContext.ReportDir = os.Getenv("TEST_JUNIT_DIR") | |
| } | |
| if len(os.Getenv("TEST_JUNIT_DIR")) > 0 { | |
| testContext.ReportDir = os.Getenv("TEST_JUNIT_DIR") | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/cloud-controller-manager-aws-tests-ext/main.go` around lines 191 - 193,
The condition currently uses len(os.Getenv("TEST_JUNIT_DIR")) >= 0 which is
always true; change the check to only set testContext.ReportDir when the
environment variable is non-empty (e.g., testDir := os.Getenv("TEST_JUNIT_DIR");
if testDir != "" { testContext.ReportDir = testDir }) so that
testContext.ReportDir is not overwritten with an empty string; update references
to os.Getenv("TEST_JUNIT_DIR") and testContext.ReportDir accordingly.
|
|
||
| ## Prerequisites | ||
|
|
||
| - Go 1.24+ |
There was a problem hiding this comment.
Go version mismatch with go.mod.
The README states "Go 1.24+" but go.mod specifies go 1.25.0. Update the prerequisite to match the actual requirement.
📝 Proposed fix
-- Go 1.24+
+- Go 1.25+📝 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.
Suggested change
| - Go 1.24+ | |
| - Go 1.25+ |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/cloud-controller-manager-aws-tests-ext/README.md` at line 39, Update the
README entry that currently says "Go 1.24+" to match the module's declared Go
version in go.mod (go 1.25.0); edit the README text so the prerequisite reflects
"Go 1.25.0" or "Go 1.25+" to align with the go directive in go.mod and avoid
version mismatch.
Contributor
Author
|
/testwith openshift/cluster-cloud-controller-manager-operator/main/e2e-aws-ovn openshift/origin#30864 |
Contributor
Author
|
/testwith openshift/cloud-provider-aws/main/e2e-aws-ovn openshift/origin#30864 |
Contributor
|
@mtulio, |
Contributor
Author
|
/test e2e-aws-ovn |
Contributor
Author
|
Fixed wrong statement in .dockerignore /test e2e-aws-ovn |
Contributor
Author
|
/testwith openshift/cluster-cloud-controller-manager-operator/main/e2e-aws-ovn openshift/origin#30864 |
|
@mtulio: This pull request references SPLAT-2672 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 task to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
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. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
cmd/cloud-controller-manager-aws-tests-ext/e2e/loadbalancer.go (1)
300-303:⚠️ Potential issue | 🟡 MinorGrammar issue in log message and error string.
"did not changed" should be "did not change".
📝 Proposed fix
if len(originalSGRules) >= len(currentSGRules) { - framework.Logf("Security group rules count did not changed: original=%d current=%d", + framework.Logf("Security group rules count did not change: original=%d current=%d", len(originalSGRules), len(currentSGRules)) - return nil, fmt.Errorf("security group rules count did not changed") + return nil, fmt.Errorf("security group rules count did not change") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/cloud-controller-manager-aws-tests-ext/e2e/loadbalancer.go` around lines 300 - 303, Fix the grammar in the security group rules message and error: update the framework.Logf call and the returned error string that currently say "did not changed" to "did not change" (the checks around originalSGRules and currentSGRules where the log and fmt.Errorf are used).
🧹 Nitpick comments (5)
Dockerfile (1)
8-12: Clarify why only the test extension binary is gzipped.The test extension binary is gzipped while other binaries (
cluster-controller-manager-operator,config-sync-controllers,azure-config-credentials-injector) are copied directly. This inconsistency may be intentional (e.g., to reduce image size for a large test binary that's not always needed), but it would be helpful to add a comment explaining the rationale.Also note that consumers will need to
gunzipthe binary before use, which should be documented in the README.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile` around lines 8 - 12, Add a brief comment in the Dockerfile next to the COPY that places cloud-controller-manager-aws-tests-ext.gz explaining why only the test extension binary is gzipped (e.g., large test-only binary kept compressed to reduce image size and because it's optional), and update the README to document that consumers must gunzip /usr/bin/cloud-controller-manager-aws-tests-ext.gz before executing; reference the COPY entry for cloud-controller-manager-aws-tests-ext.gz and the other copied binaries (cluster-controller-manager-operator, config-sync-controllers, azure-config-credentials-injector) so maintainers can see the intentional difference.cmd/cloud-controller-manager-aws-tests-ext/main.go (1)
104-124: Side effect of settingAWS_REGIONshould be documented.The
getRegionFromEnvfunction setsAWS_REGIONas a side effect when reading fromLEASED_RESOURCEorAWS_DEFAULT_REGION. Consider adding a comment to make this intentional behavior explicit, or renaming togetAndSetRegionFromEnvto make the side effect obvious.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/cloud-controller-manager-aws-tests-ext/main.go` around lines 104 - 124, getRegionFromEnv currently has the side effect of setting the AWS_REGION env var when it reads from LEASED_RESOURCE or AWS_DEFAULT_REGION; update the code to make this intentional behavior explicit by either adding a clear comment above getRegionFromEnv describing that it will set AWS_REGION as a side effect (mentioning the specific branches that call os.Setenv) or rename the function to getAndSetRegionFromEnv and update all call sites to the new name so the side effect is obvious; ensure any documentation or tests referencing getRegionFromEnv are updated to reflect the documented/renamed behavior.cmd/cloud-controller-manager-aws-tests-ext/e2e/loadbalancer.go (1)
565-576: Error handling assumes any API error means load balancer is deleted.The current logic treats any error from
getAWSLoadBalancerFromDNSNameas success (LB deleted). However, transient errors (network issues, throttling) would incorrectly mark the poll as complete. Consider distinguishing between "not found" errors and other errors.🛡️ Proposed improvement
err = wait.PollUntilContextTimeout(ctx, 5*time.Second, loadBalancerCreateTimeout, true, func(ctx context.Context) (bool, error) { foundLB, err := getAWSLoadBalancerFromDNSName(ctx, elbClient, lbDNS) if err != nil { - // Error finding LB likely means it's deleted - this is success - framework.Logf("Load balancer %s not found (likely deleted): %v", lbDNS, err) - return true, nil + // Only treat "not found" as success; propagate other errors + if strings.Contains(err.Error(), "not found") || strings.Contains(err.Error(), "LoadBalancerNotFound") { + framework.Logf("Load balancer %s not found (deleted): %v", lbDNS, err) + return true, nil + } + framework.Logf("Error querying load balancer %s: %v", lbDNS, err) + return false, nil // Retry on transient errors }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/cloud-controller-manager-aws-tests-ext/e2e/loadbalancer.go` around lines 565 - 576, The polling logic treats any error from getAWSLoadBalancerFromDNSName as a successful deletion; change it to distinguish "not found" from other errors by inspecting the returned error (e.g., check AWS error codes or use errors.Is against a sentinel not-found error returned by getAWSLoadBalancerFromDNSName). In the anonymous func passed to wait.PollUntilContextTimeout (the block using elbClient, lbDNS, loadBalancerCreateTimeout), only treat the specific not-found result as success (return true, nil); for transient/other errors log the failure and return false, nil so the poll retries (or return the error only if it should abort permanently). If needed, update getAWSLoadBalancerFromDNSName to return a clear not-found sentinel error to make this check reliable.cmd/cloud-controller-manager-aws-tests-ext/go.mod (1)
138-138:k8s.io/kubectlversion is misaligned with other k8s.io modules.The indirect dependency
k8s.io/kubectl v0.33.0is out of sync with most other Kubernetes modules pinned at v0.35.0. Bothv0.33.0andv0.35.0are stable releases and alignment is possible. Consider adding a replace directive to bring it in line with the rest.Note:
k8s.io/cloud-provider v0.34.0is similarly misaligned.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/cloud-controller-manager-aws-tests-ext/go.mod` at line 138, The go.mod lists an indirect dependency k8s.io/kubectl at v0.33.0 which is out of sync with the project’s other Kubernetes modules (mostly v0.35.0); update the module alignment by adding a replace directive that forces k8s.io/kubectl => k8s.io/kubectl v0.35.0 (and likewise add/adjust a replace for k8s.io/cloud-provider to v0.35.0 if present) so all k8s.io modules use the same v0.35.0 version; locate the k8s.io/kubectl entry in go.mod and add the replace directives accordingly.Makefile (1)
51-56: Consider usingcdwith subshell instead ofpushdwithoutpopd.The
pushdcommand changes directory but there's no correspondingpopd. While this works in this specific case, using a subshell withcdis more idiomatic in Makefiles and clearer about intent.Additionally,
LDFLAGSis undefined in the Makefile. Unlike standard Go environment variables (GOOS, GOARCH, GOPROXY), LDFLAGS must be explicitly defined. Currently,-ldflags="$(LDFLAGS)"expands to-ldflags="", which is harmless but indicates incomplete build configuration. Consider defining LDFLAGS explicitly or using hardcoded flags like-ldflags="-s -w"if appropriate for this binary.♻️ Proposed improvement
cloud-controller-manager-aws-tests-ext: - pushd cmd/cloud-controller-manager-aws-tests-ext && \ - GO111MODULE=on CGO_ENABLED=0 GOOS=$(GOOS) GOARCH=$(GOARCH) GOPROXY=$(GOPROXY) go build \ + (cd cmd/cloud-controller-manager-aws-tests-ext && \ + GO111MODULE=on CGO_ENABLED=0 GOOS=$(GOOS) GOARCH=$(GOARCH) GOPROXY=$(GOPROXY) go build \ -trimpath \ - -ldflags="$(LDFLAGS)" \ + -ldflags="-s -w" \ -o=../../bin/cloud-controller-manager-aws-tests-ext .)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 51 - 56, The cloud-controller-manager-aws-tests-ext Make target uses pushd without popd and passes an undefined LDFLAGS; change the recipe to run the build in a subshell (e.g., use (cd cmd/cloud-controller-manager-aws-tests-ext && GO111MODULE=on CGO_ENABLED=0 GOOS=$(GOOS) GOARCH=$(GOARCH) GOPROXY=$(GOPROXY) go build ... -o ... .)) instead of pushd, and add a default LDFLAGS assignment at top of the Makefile (e.g., define LDFLAGS using LDFLAGS ?= -s -w or an explicit value) so the -ldflags="$(LDFLAGS)" expansion is intentional; update the cloud-controller-manager-aws-tests-ext recipe to reference the LDFLAGS variable as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/cloud-controller-manager-aws-tests-ext/e2e/loadbalancer.go`:
- Line 80: Fix the typo in the test description string passed to It — add the
missing closing single-quote so the message reads "should have
NLBSecurityGroupMode with 'Managed' value in cloud-config"; locate the It(...)
call where the description currently is "should have NLBSecurityGroupMode with
'Managed value in cloud-config" and update that string accordingly.
---
Duplicate comments:
In `@cmd/cloud-controller-manager-aws-tests-ext/e2e/loadbalancer.go`:
- Around line 300-303: Fix the grammar in the security group rules message and
error: update the framework.Logf call and the returned error string that
currently say "did not changed" to "did not change" (the checks around
originalSGRules and currentSGRules where the log and fmt.Errorf are used).
---
Nitpick comments:
In `@cmd/cloud-controller-manager-aws-tests-ext/e2e/loadbalancer.go`:
- Around line 565-576: The polling logic treats any error from
getAWSLoadBalancerFromDNSName as a successful deletion; change it to distinguish
"not found" from other errors by inspecting the returned error (e.g., check AWS
error codes or use errors.Is against a sentinel not-found error returned by
getAWSLoadBalancerFromDNSName). In the anonymous func passed to
wait.PollUntilContextTimeout (the block using elbClient, lbDNS,
loadBalancerCreateTimeout), only treat the specific not-found result as success
(return true, nil); for transient/other errors log the failure and return false,
nil so the poll retries (or return the error only if it should abort
permanently). If needed, update getAWSLoadBalancerFromDNSName to return a clear
not-found sentinel error to make this check reliable.
In `@cmd/cloud-controller-manager-aws-tests-ext/go.mod`:
- Line 138: The go.mod lists an indirect dependency k8s.io/kubectl at v0.33.0
which is out of sync with the project’s other Kubernetes modules (mostly
v0.35.0); update the module alignment by adding a replace directive that forces
k8s.io/kubectl => k8s.io/kubectl v0.35.0 (and likewise add/adjust a replace for
k8s.io/cloud-provider to v0.35.0 if present) so all k8s.io modules use the same
v0.35.0 version; locate the k8s.io/kubectl entry in go.mod and add the replace
directives accordingly.
In `@cmd/cloud-controller-manager-aws-tests-ext/main.go`:
- Around line 104-124: getRegionFromEnv currently has the side effect of setting
the AWS_REGION env var when it reads from LEASED_RESOURCE or AWS_DEFAULT_REGION;
update the code to make this intentional behavior explicit by either adding a
clear comment above getRegionFromEnv describing that it will set AWS_REGION as a
side effect (mentioning the specific branches that call os.Setenv) or rename the
function to getAndSetRegionFromEnv and update all call sites to the new name so
the side effect is obvious; ensure any documentation or tests referencing
getRegionFromEnv are updated to reflect the documented/renamed behavior.
In `@Dockerfile`:
- Around line 8-12: Add a brief comment in the Dockerfile next to the COPY that
places cloud-controller-manager-aws-tests-ext.gz explaining why only the test
extension binary is gzipped (e.g., large test-only binary kept compressed to
reduce image size and because it's optional), and update the README to document
that consumers must gunzip /usr/bin/cloud-controller-manager-aws-tests-ext.gz
before executing; reference the COPY entry for
cloud-controller-manager-aws-tests-ext.gz and the other copied binaries
(cluster-controller-manager-operator, config-sync-controllers,
azure-config-credentials-injector) so maintainers can see the intentional
difference.
In `@Makefile`:
- Around line 51-56: The cloud-controller-manager-aws-tests-ext Make target uses
pushd without popd and passes an undefined LDFLAGS; change the recipe to run the
build in a subshell (e.g., use (cd cmd/cloud-controller-manager-aws-tests-ext &&
GO111MODULE=on CGO_ENABLED=0 GOOS=$(GOOS) GOARCH=$(GOARCH) GOPROXY=$(GOPROXY) go
build ... -o ... .)) instead of pushd, and add a default LDFLAGS assignment at
top of the Makefile (e.g., define LDFLAGS using LDFLAGS ?= -s -w or an explicit
value) so the -ldflags="$(LDFLAGS)" expansion is intentional; update the
cloud-controller-manager-aws-tests-ext recipe to reference the LDFLAGS variable
as before.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 12e50d26-0815-44fc-abda-503961d9f9d7
⛔ Files ignored due to path filters (1)
cmd/cloud-controller-manager-aws-tests-ext/go.sumis excluded by!**/*.sum
📒 Files selected for processing (8)
.dockerignoreDockerfileMakefilecmd/cloud-controller-manager-aws-tests-ext/README.mdcmd/cloud-controller-manager-aws-tests-ext/e2e/helper.gocmd/cloud-controller-manager-aws-tests-ext/e2e/loadbalancer.gocmd/cloud-controller-manager-aws-tests-ext/go.modcmd/cloud-controller-manager-aws-tests-ext/main.go
🚧 Files skipped from review as they are similar to previous changes (2)
- cmd/cloud-controller-manager-aws-tests-ext/e2e/helper.go
- .dockerignore
| // - Configuration includes: NLBSecurityGroupMode set to 'Managed' | ||
| // - The test must fail if the feature gate is enabled and the configuration does not include NLBSecurityGroupMode set to 'Managed' | ||
| // - The test must skip if the feature gate is not enabled | ||
| It("should have NLBSecurityGroupMode with 'Managed value in cloud-config", func(ctx context.Context) { |
There was a problem hiding this comment.
Minor typo in test description.
There's a missing closing quote in the test description: 'Managed value should be 'Managed' value.
📝 Proposed fix
- It("should have NLBSecurityGroupMode with 'Managed value in cloud-config", func(ctx context.Context) {
+ It("should have NLBSecurityGroupMode with 'Managed' value in cloud-config", func(ctx context.Context) {📝 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.
Suggested change
| It("should have NLBSecurityGroupMode with 'Managed value in cloud-config", func(ctx context.Context) { | |
| It("should have NLBSecurityGroupMode with 'Managed' value in cloud-config", func(ctx context.Context) { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/cloud-controller-manager-aws-tests-ext/e2e/loadbalancer.go` at line 80,
Fix the typo in the test description string passed to It — add the missing
closing single-quote so the message reads "should have NLBSecurityGroupMode with
'Managed' value in cloud-config"; locate the It(...) call where the description
currently is "should have NLBSecurityGroupMode with 'Managed value in
cloud-config" and update that string accordingly.
Contributor
Author
|
/testwith openshift/cluster-cloud-controller-manager-operator/main/e2e-aws-ovn openshift/origin#30864,openshift/cloud-provider-aws#133 |
Contributor
Author
|
/testwith openshift/cluster-cloud-controller-manager-operator/main/e2e-aws-ovn openshift/origin#30864 openshift/cloud-provider-aws#133 |
|
@mtulio: This pull request references SPLAT-2672 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 task to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
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. |
Contributor
|
@mtulio: all tests passed! Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
Moving the OTE binary from CCM-AWS to CCCMO project.
Contributor
Author
|
I've found two issues in my last attempt:
/testwith openshift/cluster-cloud-controller-manager-operator/main/e2e-aws-ovn openshift/origin#30864 openshift/cloud-provider-aws#133 |
|
@mtulio: This pull request references SPLAT-2672 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 story to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
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. |
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (4)
cmd/cloud-controller-manager-aws-tests-ext/README.md (2)
39-39:⚠️ Potential issue | 🟡 MinorUpdate the Go prerequisite to match
go.mod.
go.modfor this module is set togo 1.25.0, soGo 1.24+sends readers to the wrong toolchain.📝 Proposed fix
-- Go 1.24+ +- Go 1.25+🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/cloud-controller-manager-aws-tests-ext/README.md` at line 39, The README's Go prerequisite is outdated; update the Go version requirement text (the line currently reading "Go 1.24+") to match the module's go.mod (which specifies go 1.25.0) by changing it to "Go 1.25+" so documentation aligns with the actual toolchain required.
1-4:⚠️ Potential issue | 🟡 MinorThe documented binary name and commands still don't match the build output.
The Makefile builds
bin/cloud-controller-manager-aws-tests-ext, but the README examples still useaws-cloud-controller-manager-tests-extand the old make invocation, so the copy-paste path here won't work.📝 Proposed fix
-# aws-cloud-controller-manager-tests-ext (OTE binary) +# cloud-controller-manager-aws-tests-ext (OTE binary) @@ ```sh -make -f openshift-hack/Makefile aws-cloud-controller-manager-tests-ext +make cloud-controller-manager-aws-tests-ext-The binary will be created at the repository root:
./aws-cloud-controller-manager-tests-ext
+The binary will be created at:./bin/cloud-controller-manager-aws-tests-ext
@@-$ ./aws-cloud-controller-manager-tests-ext list --topology=HighAvailability --platform=aws | jq .[].name +$ ./bin/cloud-controller-manager-aws-tests-ext list --topology=HighAvailability --platform=aws | jq .[].name @@ ```sh $ export KUBECONFIG=/path/to/kubeconfig -$ ./aws-cloud-controller-manager-tests-ext run-test "[cloud-provider-aws-e2e-openshift] loadbalancer NLB feature AWSServiceLBNetworkSecurityGroup should have security groups attached to default ingress controller NLB [Suite:openshift/conformance/parallel]" +$ ./bin/cloud-controller-manager-aws-tests-ext run-test "[cloud-provider-aws-e2e-openshift] loadbalancer NLB feature AWSServiceLBNetworkSecurityGroup should have security groups attached to default ingress controller NLB [Suite:openshift/conformance/parallel]"</details> Also applies to: 46-50, 56-68 <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@cmd/cloud-controller-manager-aws-tests-ext/README.mdaround lines 1 - 4,
README references the old binary name and make target
(aws-cloud-controller-manager-tests-ext) which don't match the build output;
update all occurrences to the actual build target and path: change the make
invocation to "make cloud-controller-manager-aws-tests-ext", update binary paths
from "./aws-cloud-controller-manager-tests-ext" to
"./bin/cloud-controller-manager-aws-tests-ext", and adjust all example commands
(list, run-test) accordingly so the documented commands match the built artifact
name and location.</details> </blockquote></details> <details> <summary>cmd/cloud-controller-manager-aws-tests-ext/e2e/loadbalancer.go (1)</summary><blockquote> `297-303`: _⚠️ Potential issue_ | _🟠 Major_ **Return SG lookup errors from `Eventually` instead of asserting.** `framework.ExpectNoError` aborts the spec on the first transient AWS failure, so this poll never retries. <details> <summary>🐛 Proposed fix</summary> ```diff currentSGRules, err := getAWSSecurityGroupRules(ctx, ec2Client, foundLB.SecurityGroups) - framework.ExpectNoError(err, "failed to get security group rules to calculate the diff") + if err != nil { + framework.Logf("failed to get security group rules: %v", err) + return nil, err + } if len(originalSGRules) >= len(currentSGRules) { - framework.Logf("Security group rules count did not changed: original=%d current=%d", + framework.Logf("Security group rules count did not change: original=%d current=%d", len(originalSGRules), len(currentSGRules)) - return nil, fmt.Errorf("security group rules count did not changed") + return nil, fmt.Errorf("security group rules count did not change") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/cloud-controller-manager-aws-tests-ext/e2e/loadbalancer.go` around lines 297 - 303, The call to framework.ExpectNoError inside the Eventually poll (around getAWSSecurityGroupRules) aborts the test on transient AWS errors; replace that assertion with returning the error so the poll can retry: in the block that calls getAWSSecurityGroupRules(ctx, ec2Client, foundLB.SecurityGroups) handle the err with if err != nil { return nil, err } instead of framework.ExpectNoError, and keep returning an error when len(originalSGRules) >= len(currentSGRules) (e.g., return nil, fmt.Errorf(...)) so Eventually receives errors and continues polling; update references to getAWSSecurityGroupRules, originalSGRules, currentSGRules, and the surrounding Eventually closure accordingly.cmd/cloud-controller-manager-aws-tests-ext/go.mod (1)
130-138:⚠️ Potential issue | 🟠 MajorResolve the remaining Kubernetes version skew.
k8s.io/cloud-provideris still atv0.34.0while the rest of this module is pinned/replaced to thev0.35.xline. That skew is where cross-module API/type mismatches usually show up.🧩 Proposed fix
replace ( github.com/onsi/ginkgo/v2 => github.com/openshift/onsi-ginkgo/v2 v2.6.1-0.20260303184444-1cc650aa0565 k8s.io/apiextensions-apiserver => k8s.io/apiextensions-apiserver v0.35.0 + k8s.io/cloud-provider => k8s.io/cloud-provider v0.35.0 // Required for k8s.io/kubernetes v1.35.0 k8s.io/cluster-bootstrap => k8s.io/cluster-bootstrap v0.35.0#!/bin/bash # Inspect Kubernetes module versions declared in this go.mod. python - <<'PY' import re from pathlib import Path text = Path("cmd/cloud-controller-manager-aws-tests-ext/go.mod").read_text().splitlines() for line in text: m = re.match(r'\s*(k8s\.io/[^\s]+)\s+v([0-9]+\.[0-9]+\.[0-9]+)', line) if m: print(f"{m.group(1)} {m.group(2)}") PY🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/cloud-controller-manager-aws-tests-ext/go.mod` around lines 130 - 138, k8s.io/cloud-provider is pinned to v0.34.0 while the rest of the Kubernetes modules target the v0.35.x line; update the module declaration for k8s.io/cloud-provider to the matching v0.35.0 (or the same v0.35.x version used elsewhere), then run go get k8s.io/cloud-provider@v0.35.0 (or the exact v0.35.x version) and go mod tidy to refresh go.sum and ensure no skew; look for the k8s.io/cloud-provider entry in the go.mod and update it to match the other k8s.io/* versions.
🧹 Nitpick comments (1)
Makefile (1)
39-40: Declarebuildphony.A file or directory named
buildat the repo root will short-circuit this target. Adding.PHONY: buildkeeps the build path deterministic.♻️ Proposed fix
+.PHONY: build build: operator config-sync-controllers azure-config-credentials-injector cloud-controller-manager-aws-tests-ext🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 39 - 40, The Makefile's build target can be shadowed by a file/directory named "build"; add a phony declaration for it by including ".PHONY: build" (or by adding "build" to an existing .PHONY line) so the make target always runs regardless of filesystem entries; update the Makefile near the other .PHONY declarations to reference the build target name.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/cloud-controller-manager-aws-tests-ext/e2e/loadbalancer.go`:
- Around line 119-141: After creating the NLB service with createServiceNLB,
immediately register a cleanup action so the service (and its NLB/security
groups) are removed even if the test fails early; use framework.AddCleanupAction
(or the suite's cleanup helper) to call
deleteServiceAndWaitForLoadBalancerDeletion (or at minimum jig.DeleteService)
with the returned jig and service info (svc and/or
svc.Status.LoadBalancer.Ingress[0].Hostname) instead of only deleting at the end
of the test; place this registration right after createServiceNLB returns
(referencing createServiceNLB, svc, jig, and
deleteServiceAndWaitForLoadBalancerDeletion) so resources are always cleaned up
on failure.
- Around line 467-476: The test incorrectly asserts the exact multiset of ports
by using ConsistOf on expectedPorts, which fails when SGs have duplicates or
extra managed rules; update the assertion to check that both required ports (80
and 443) are present rather than matching the exact set: collect ports from
currentSGRules into expectedPorts (already done) and replace the ConsistOf
assertion with a presence check such as
Expect(expectedPorts).To(ContainElements(int32(80), int32(443))) or assert that
a set of ports contains both 80 and 443 so duplicates/extra rules don't break
the test; reference expectedPorts and currentSGRules when locating the change.
- Around line 202-213: The poll callback dereferences lb.State without ensuring
lb is non-nil; update the callback used in wait.PollUntilContextTimeout so it
checks for lb != nil before accessing lb.State (from
getAWSLoadBalancerFromDNSName), e.g. only log lb.State when lb and lb.State are
non-nil and otherwise log that the load balancer is nil/not found or skip the
state access; modify the framework.Logf call at the end of the callback to guard
against nil lb and avoid a panic.
- Around line 292-294: The check that logs the number of security groups
dereferences foundLB even when it's nil; change the logic in the load balancer
check (the if block that uses foundLB and foundLB.SecurityGroups) to avoid
accessing foundLB.SecurityGroups when foundLB == nil — e.g. determine sgCount
conditionally (sgCount := 0; if foundLB != nil { sgCount =
len(foundLB.SecurityGroups) }) or split the nil check before using len, then use
sgCount in framework.Logf and keep the existing error return; this ensures
Eventually can retry without panicking.
- Around line 565-570: The current wait.PollUntilContextTimeout callback treats
any error from getAWSLoadBalancerFromDNSName as a successful deletion; change it
to only consider deletion successful when the call returns a
LoadBalancerNotFoundException (AWS error code for a missing LB) or when foundLB
== nil, and for all other errors return false, err so the poll will retry; in
practice update the error handling in the anonymous function passed to
wait.PollUntilContextTimeout to inspect the AWS SDK error (e.g., via
awserr.Error or error.Code()/ErrorCode/Is depending on the SDK) for the
LoadBalancerNotFoundException value and only then return true, nil, otherwise
return false, err, keeping lbDNS, elbClient and getAWSLoadBalancerFromDNSName as
reference points.
In `@cmd/cloud-controller-manager-aws-tests-ext/README.md`:
- Around line 8-9: Update the README.md directory layout to match the repository
structure: replace references to
tests/aws-cloud-controller-manager-tests-ext/... with
cmd/cloud-controller-manager-aws-tests-ext/... and include the additional files
present in the repo (e.g., go.mod and README.md at the top-level of that
module). Edit the README text that lists the tree (the section around the
example layout and lines 25-34) so it shows
"cmd/cloud-controller-manager-aws-tests-ext/" as the root, includes the
e2e/loadbalancer.go and e2e/helper.go entries, and lists main.go, go.mod, and
README.md to accurately reflect the current code location.
In `@Makefile`:
- Around line 51-56: The Makefile recipe for target
cloud-controller-manager-aws-tests-ext uses the Bash builtin pushd which fails
under /bin/sh in CI; replace the pushd/popd usage with a POSIX-safe change
directory by using cd into cmd/cloud-controller-manager-aws-tests-ext (or wrap
the build in a subshell with (cd cmd/cloud-controller-manager-aws-tests-ext &&
GO111MODULE=... go build ...)) so the build runs under /bin/sh without requiring
Bash.
---
Duplicate comments:
In `@cmd/cloud-controller-manager-aws-tests-ext/e2e/loadbalancer.go`:
- Around line 297-303: The call to framework.ExpectNoError inside the Eventually
poll (around getAWSSecurityGroupRules) aborts the test on transient AWS errors;
replace that assertion with returning the error so the poll can retry: in the
block that calls getAWSSecurityGroupRules(ctx, ec2Client,
foundLB.SecurityGroups) handle the err with if err != nil { return nil, err }
instead of framework.ExpectNoError, and keep returning an error when
len(originalSGRules) >= len(currentSGRules) (e.g., return nil, fmt.Errorf(...))
so Eventually receives errors and continues polling; update references to
getAWSSecurityGroupRules, originalSGRules, currentSGRules, and the surrounding
Eventually closure accordingly.
In `@cmd/cloud-controller-manager-aws-tests-ext/go.mod`:
- Around line 130-138: k8s.io/cloud-provider is pinned to v0.34.0 while the rest
of the Kubernetes modules target the v0.35.x line; update the module declaration
for k8s.io/cloud-provider to the matching v0.35.0 (or the same v0.35.x version
used elsewhere), then run go get k8s.io/cloud-provider@v0.35.0 (or the exact
v0.35.x version) and go mod tidy to refresh go.sum and ensure no skew; look for
the k8s.io/cloud-provider entry in the go.mod and update it to match the other
k8s.io/* versions.
In `@cmd/cloud-controller-manager-aws-tests-ext/README.md`:
- Line 39: The README's Go prerequisite is outdated; update the Go version
requirement text (the line currently reading "Go 1.24+") to match the module's
go.mod (which specifies go 1.25.0) by changing it to "Go 1.25+" so documentation
aligns with the actual toolchain required.
- Around line 1-4: README references the old binary name and make target
(aws-cloud-controller-manager-tests-ext) which don't match the build output;
update all occurrences to the actual build target and path: change the make
invocation to "make cloud-controller-manager-aws-tests-ext", update binary paths
from "./aws-cloud-controller-manager-tests-ext" to
"./bin/cloud-controller-manager-aws-tests-ext", and adjust all example commands
(list, run-test) accordingly so the documented commands match the built artifact
name and location.
---
Nitpick comments:
In `@Makefile`:
- Around line 39-40: The Makefile's build target can be shadowed by a
file/directory named "build"; add a phony declaration for it by including
".PHONY: build" (or by adding "build" to an existing .PHONY line) so the make
target always runs regardless of filesystem entries; update the Makefile near
the other .PHONY declarations to reference the build target name.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d93b3012-1a18-463c-b1a0-2caf39ccd0e3
⛔ Files ignored due to path filters (1)
cmd/cloud-controller-manager-aws-tests-ext/go.sumis excluded by!**/*.sum
📒 Files selected for processing (8)
.dockerignoreDockerfileMakefilecmd/cloud-controller-manager-aws-tests-ext/README.mdcmd/cloud-controller-manager-aws-tests-ext/e2e/helper.gocmd/cloud-controller-manager-aws-tests-ext/e2e/loadbalancer.gocmd/cloud-controller-manager-aws-tests-ext/go.modcmd/cloud-controller-manager-aws-tests-ext/main.go
🚧 Files skipped from review as they are similar to previous changes (3)
- cmd/cloud-controller-manager-aws-tests-ext/main.go
- .dockerignore
- cmd/cloud-controller-manager-aws-tests-ext/e2e/helper.go
Comment on lines
+119
to
+141
| By("creating test service and deployment configuration") | ||
| serviceName := "nlb-sg-crt" | ||
| svc, jig, err := createServiceNLB(ctx, cs, ns, serviceName, map[int32]int32{80: 8080}) | ||
| framework.ExpectNoError(err, "failed to create NLB service load balancer") | ||
|
|
||
| foundLB, err := getNLBMetaFromName(ctx, cs, ns, serviceName) | ||
| framework.ExpectNoError(err, "failed to get NLB metadata") | ||
| Expect(foundLB).NotTo(BeNil(), "found load balancer is nil") | ||
|
|
||
| By("verifying security groups are attached to the NLB") | ||
| Expect(len(foundLB.SecurityGroups)).To(BeNumerically(">", 0), | ||
| "load balancer should have security groups attached when NLBSecurityGroupMode = Managed") | ||
|
|
||
| framework.Logf("Successfully validated that load balancer has %d security group(s) attached", len(foundLB.SecurityGroups)) | ||
| for i, sg := range foundLB.SecurityGroups { | ||
| framework.Logf(" Security Group %d: %s", i+1, sg) | ||
| } | ||
|
|
||
| lbDNS := svc.Status.LoadBalancer.Ingress[0].Hostname | ||
| framework.Logf("Load balancer DNS: %s", lbDNS) | ||
|
|
||
| err = deleteServiceAndWaitForLoadBalancerDeletion(ctx, jig, lbDNS) | ||
| framework.ExpectNoError(err, "failed to delete service and wait for load balancer deletion") |
There was a problem hiding this comment.
Register cleanup immediately after provisioning.
These specs only clean up at the tail. Any failure before the final delete leaks the NLB and managed security groups into the AWS account.
🧹 Proposed fix pattern
lbDNS := svc.Status.LoadBalancer.Ingress[0].Hostname
+ DeferCleanup(func(ctx context.Context) {
+ err := deleteServiceAndWaitForLoadBalancerDeletion(ctx, jig, lbDNS)
+ framework.ExpectNoError(err, "failed to delete service and wait for load balancer deletion")
+ })
framework.Logf("Load balancer DNS: %s", lbDNS)
-
- err = deleteServiceAndWaitForLoadBalancerDeletion(ctx, jig, lbDNS)
- framework.ExpectNoError(err, "failed to delete service and wait for load balancer deletion")
})Also applies to: 243-331, 435-480
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/cloud-controller-manager-aws-tests-ext/e2e/loadbalancer.go` around lines
119 - 141, After creating the NLB service with createServiceNLB, immediately
register a cleanup action so the service (and its NLB/security groups) are
removed even if the test fails early; use framework.AddCleanupAction (or the
suite's cleanup helper) to call deleteServiceAndWaitForLoadBalancerDeletion (or
at minimum jig.DeleteService) with the returned jig and service info (svc and/or
svc.Status.LoadBalancer.Ingress[0].Hostname) instead of only deleting at the end
of the test; place this registration right after createServiceNLB returns
(referencing createServiceNLB, svc, jig, and
deleteServiceAndWaitForLoadBalancerDeletion) so resources are always cleaned up
on failure.
Comment on lines
+202
to
+213
| err = wait.PollUntilContextTimeout(ctx, 10*time.Second, 3*time.Minute, true, func(pollCtx context.Context) (bool, error) { | ||
| lb, err := getAWSLoadBalancerFromDNSName(pollCtx, elbClient, lbDNS) | ||
| if err != nil { | ||
| framework.Logf("Failed to find load balancer with DNS %s: %v", lbDNS, err) | ||
| return false, nil | ||
| } | ||
| if lb != nil && lb.State != nil && lb.State.Code == elbv2types.LoadBalancerStateEnumActive { | ||
| foundLB = lb | ||
| return true, nil | ||
| } | ||
| framework.Logf("Load balancer not yet active, current state: %v", lb.State) | ||
| return false, nil |
There was a problem hiding this comment.
Guard the nil load balancer before logging its state.
If getAWSLoadBalancerFromDNSName returns nil, nil, Line 212 dereferences lb.State and panics instead of retrying.
🐛 Proposed fix
if lb != nil && lb.State != nil && lb.State.Code == elbv2types.LoadBalancerStateEnumActive {
foundLB = lb
return true, nil
}
- framework.Logf("Load balancer not yet active, current state: %v", lb.State)
+ if lb == nil {
+ framework.Logf("Load balancer %s not returned yet", lbDNS)
+ return false, nil
+ }
+ framework.Logf("Load balancer not yet active, current state: %v", lb.State)
return false, nil🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/cloud-controller-manager-aws-tests-ext/e2e/loadbalancer.go` around lines
202 - 213, The poll callback dereferences lb.State without ensuring lb is
non-nil; update the callback used in wait.PollUntilContextTimeout so it checks
for lb != nil before accessing lb.State (from getAWSLoadBalancerFromDNSName),
e.g. only log lb.State when lb and lb.State are non-nil and otherwise log that
the load balancer is nil/not found or skip the state access; modify the
framework.Logf call at the end of the callback to guard against nil lb and avoid
a panic.
Comment on lines
+292
to
+294
| if foundLB == nil || len(foundLB.SecurityGroups) == 0 { | ||
| framework.Logf("Load balancer has no security groups attached, expected=%d", len(foundLB.SecurityGroups)) | ||
| return nil, fmt.Errorf("load balancer has no security groups attached") |
There was a problem hiding this comment.
Don't dereference foundLB in the retry path.
When foundLB is nil, Line 293 evaluates len(foundLB.SecurityGroups) and panics, so Eventually never gets a chance to retry.
🐛 Proposed fix
- if foundLB == nil || len(foundLB.SecurityGroups) == 0 {
- framework.Logf("Load balancer has no security groups attached, expected=%d", len(foundLB.SecurityGroups))
+ if foundLB == nil {
+ framework.Logf("Load balancer not found yet")
+ return nil, fmt.Errorf("load balancer not found yet")
+ }
+ if len(foundLB.SecurityGroups) == 0 {
+ framework.Logf("Load balancer has no security groups attached")
return nil, fmt.Errorf("load balancer has no security groups attached")
}📝 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.
Suggested change
| if foundLB == nil || len(foundLB.SecurityGroups) == 0 { | |
| framework.Logf("Load balancer has no security groups attached, expected=%d", len(foundLB.SecurityGroups)) | |
| return nil, fmt.Errorf("load balancer has no security groups attached") | |
| if foundLB == nil { | |
| framework.Logf("Load balancer not found yet") | |
| return nil, fmt.Errorf("load balancer not found yet") | |
| } | |
| if len(foundLB.SecurityGroups) == 0 { | |
| framework.Logf("Load balancer has no security groups attached") | |
| return nil, fmt.Errorf("load balancer has no security groups attached") | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/cloud-controller-manager-aws-tests-ext/e2e/loadbalancer.go` around lines
292 - 294, The check that logs the number of security groups dereferences
foundLB even when it's nil; change the logic in the load balancer check (the if
block that uses foundLB and foundLB.SecurityGroups) to avoid accessing
foundLB.SecurityGroups when foundLB == nil — e.g. determine sgCount
conditionally (sgCount := 0; if foundLB != nil { sgCount =
len(foundLB.SecurityGroups) }) or split the nil check before using len, then use
sgCount in framework.Logf and keep the existing error return; this ensures
Eventually can retry without panicking.
Comment on lines
+467
to
+476
| expectedPorts := []int32{} | ||
| for _, rule := range currentSGRules { | ||
| if rule.ToPort == nil { | ||
| continue | ||
| } | ||
| if *rule.ToPort == 80 || *rule.ToPort == 443 { | ||
| expectedPorts = append(expectedPorts, *rule.ToPort) | ||
| } | ||
| } | ||
| Expect(expectedPorts).To(ConsistOf([]int32{80, 443}), "security groups should have rules for both ports 80 and 443") |
There was a problem hiding this comment.
Check for required ports, not an exact SG rule multiset.
Security groups can legitimately contain duplicate or extra managed rules. ConsistOf([]int32{80, 443}) makes this test fail even when both required ports are present.
🐛 Proposed fix
- expectedPorts := []int32{}
+ requiredPorts := map[int32]bool{80: false, 443: false}
for _, rule := range currentSGRules {
if rule.ToPort == nil {
continue
}
- if *rule.ToPort == 80 || *rule.ToPort == 443 {
- expectedPorts = append(expectedPorts, *rule.ToPort)
+ if _, ok := requiredPorts[*rule.ToPort]; ok {
+ requiredPorts[*rule.ToPort] = true
}
}
- Expect(expectedPorts).To(ConsistOf([]int32{80, 443}), "security groups should have rules for both ports 80 and 443")
+ Expect(requiredPorts[80]).To(BeTrue(), "security groups should have a rule for port 80")
+ Expect(requiredPorts[443]).To(BeTrue(), "security groups should have a rule for port 443")📝 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.
Suggested change
| expectedPorts := []int32{} | |
| for _, rule := range currentSGRules { | |
| if rule.ToPort == nil { | |
| continue | |
| } | |
| if *rule.ToPort == 80 || *rule.ToPort == 443 { | |
| expectedPorts = append(expectedPorts, *rule.ToPort) | |
| } | |
| } | |
| Expect(expectedPorts).To(ConsistOf([]int32{80, 443}), "security groups should have rules for both ports 80 and 443") | |
| requiredPorts := map[int32]bool{80: false, 443: false} | |
| for _, rule := range currentSGRules { | |
| if rule.ToPort == nil { | |
| continue | |
| } | |
| if _, ok := requiredPorts[*rule.ToPort]; ok { | |
| requiredPorts[*rule.ToPort] = true | |
| } | |
| } | |
| Expect(requiredPorts[80]).To(BeTrue(), "security groups should have a rule for port 80") | |
| Expect(requiredPorts[443]).To(BeTrue(), "security groups should have a rule for port 443") |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/cloud-controller-manager-aws-tests-ext/e2e/loadbalancer.go` around lines
467 - 476, The test incorrectly asserts the exact multiset of ports by using
ConsistOf on expectedPorts, which fails when SGs have duplicates or extra
managed rules; update the assertion to check that both required ports (80 and
443) are present rather than matching the exact set: collect ports from
currentSGRules into expectedPorts (already done) and replace the ConsistOf
assertion with a presence check such as
Expect(expectedPorts).To(ContainElements(int32(80), int32(443))) or assert that
a set of ports contains both 80 and 443 so duplicates/extra rules don't break
the test; reference expectedPorts and currentSGRules when locating the change.
Comment on lines
+565
to
+570
| err = wait.PollUntilContextTimeout(ctx, 5*time.Second, loadBalancerCreateTimeout, true, func(ctx context.Context) (bool, error) { | ||
| foundLB, err := getAWSLoadBalancerFromDNSName(ctx, elbClient, lbDNS) | ||
| if err != nil { | ||
| // Error finding LB likely means it's deleted - this is success | ||
| framework.Logf("Load balancer %s not found (likely deleted): %v", lbDNS, err) | ||
| return true, nil |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect how getAWSLoadBalancerFromDNSName distinguishes "not found" from transient ELB errors.
sed -n '1,260p' cmd/cloud-controller-manager-aws-tests-ext/e2e/helper.go
rg -n -C3 'func getAWSLoadBalancerFromDNSName|DescribeLoadBalancers|LoadBalancerNotFound|NotFound' cmd/cloud-controller-manager-aws-tests-ext/e2eRepository: openshift/cluster-cloud-controller-manager-operator
Length of output: 10096
🏁 Script executed:
rg -n 'NoLoadBalancersFound|LoadBalancerNotFound|not found.*load.*balancer' cmd/cloud-controller-manager-aws-tests-ext/e2eRepository: openshift/cluster-cloud-controller-manager-operator
Length of output: 77
🏁 Script executed:
rg -n 'elbv2.*error|LoadBalancer.*error' cmd/cloud-controller-manager-aws-tests-ext/e2eRepository: openshift/cluster-cloud-controller-manager-operator
Length of output: 819
🌐 Web query:
AWS Go SDK v2 ELBv2 DescribeLoadBalancers error handling "not found"
💡 Result:
In AWS SDK for Go v2, elbv2.DescribeLoadBalancers returns a modeled service error when the LB doesn’t exist: *types.LoadBalancerNotFoundException (“The specified load balancer does not exist.”) [2]. The SDK may wrap it (commonly in *smithy.OperationError), so use errors.As to unwrap and match the modeled type (or fall back to smithy.APIError + ErrorCode()). [1]
import (
"context"
"errors"
"github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2"
elbtypes "github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2/types"
"github.com/aws/smithy-go"
)
out, err := c.DescribeLoadBalancers(ctx, &elasticloadbalancingv2.DescribeLoadBalancersInput{
LoadBalancerArns: []string{lbArn},
})
_ = out
if err != nil {
// Preferred: match the modeled "not found" exception
var nf *elbtypes.LoadBalancerNotFoundException
if errors.As(err, &nf) {
// handle "not found"
return
}
// Generic fallback (works for modeled + unmodeled AWS errors)
var apiErr smithy.APIError
if errors.As(err, &apiErr) && apiErr.ErrorCode() == "LoadBalancerNotFoundException" {
// handle "not found"
return
}
// other error
return
}Sources: AWS Go v2 error handling guidance [1]; LoadBalancerNotFoundException type docs [2].
🏁 Script executed:
sed -n '560,580p' cmd/cloud-controller-manager-aws-tests-ext/e2e/loadbalancer.goRepository: openshift/cluster-cloud-controller-manager-operator
Length of output: 952
🏁 Script executed:
# Check if there's any existing ELBv2 error type checking in the codebase
rg -n 'LoadBalancerNotFoundException|errors\.As' cmd/cloud-controller-manager-aws-tests-ext/e2eRepository: openshift/cluster-cloud-controller-manager-operator
Length of output: 77
Treat load balancer "not found" errors separately from transient AWS errors.
The code currently treats all errors from getAWSLoadBalancerFromDNSName() as successful deletion. This conflates "LB doesn't exist" (legitimate success) with throttling, auth, and transient AWS failures, which can hide leaked load balancers and make the cleanup assertion meaningless.
Instead, retry on transient errors and only consider the load balancer successfully deleted when:
getAWSLoadBalancerFromDNSName()returnsLoadBalancerNotFoundException(the LB genuinely doesn't exist)foundLB == nil(LB lookup succeeded but found no match)
Distinguish between these cases using the AWS SDK error type. For other errors, return false, err to retry the poll.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/cloud-controller-manager-aws-tests-ext/e2e/loadbalancer.go` around lines
565 - 570, The current wait.PollUntilContextTimeout callback treats any error
from getAWSLoadBalancerFromDNSName as a successful deletion; change it to only
consider deletion successful when the call returns a
LoadBalancerNotFoundException (AWS error code for a missing LB) or when foundLB
== nil, and for all other errors return false, err so the poll will retry; in
practice update the error handling in the anonymous function passed to
wait.PollUntilContextTimeout to inspect the AWS SDK error (e.g., via
awserr.Error or error.Code()/ErrorCode/Is depending on the SDK) for the
LoadBalancerNotFoundException value and only then return true, nil, otherwise
return false, err, keeping lbDNS, elbClient and getAWSLoadBalancerFromDNSName as
reference points.
Comment on lines
+8
to
+9
| OpenShift-specific (downstream) tests live under `tests/aws-cloud-controller-manager-tests-ext/e2e` | ||
| and are added to the list of tests executed by `openshift-tests`. |
There was a problem hiding this comment.
The documented directory layout doesn't match this repo.
This README points readers to tests/aws-cloud-controller-manager-tests-ext/..., but the code added in this PR lives under cmd/cloud-controller-manager-aws-tests-ext/....
📝 Proposed fix
-OpenShift-specific (downstream) tests live under `tests/aws-cloud-controller-manager-tests-ext/e2e`
+OpenShift-specific (downstream) tests in this repository live under `cmd/cloud-controller-manager-aws-tests-ext/e2e`
@@
-```
-tests/aws-cloud-controller-manager-tests-ext/
-├── e2e/
-│ ├── loadbalancer.go # OpenShift-specific load balancer tests
-│ └── helper.go # Helper functions (AWS clients, feature gate checks, etc.)
-├── main.go # Test binary entrypoint
-└── README.md
-```
+```text
+cmd/cloud-controller-manager-aws-tests-ext/
+├── e2e/
+│ ├── loadbalancer.go # OpenShift-specific load balancer tests
+│ └── helper.go # Helper functions (AWS clients, feature gate checks, etc.)
+├── main.go # Test binary entrypoint
+├── go.mod
+└── README.md
+```Also applies to: 25-34
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/cloud-controller-manager-aws-tests-ext/README.md` around lines 8 - 9,
Update the README.md directory layout to match the repository structure: replace
references to tests/aws-cloud-controller-manager-tests-ext/... with
cmd/cloud-controller-manager-aws-tests-ext/... and include the additional files
present in the repo (e.g., go.mod and README.md at the top-level of that
module). Edit the README text that lists the tree (the section around the
example layout and lines 25-34) so it shows
"cmd/cloud-controller-manager-aws-tests-ext/" as the root, includes the
e2e/loadbalancer.go and e2e/helper.go entries, and lists main.go, go.mod, and
README.md to accurately reflect the current code location.
Comment on lines
+51
to
+56
| cloud-controller-manager-aws-tests-ext: | ||
| pushd cmd/cloud-controller-manager-aws-tests-ext && \ | ||
| GO111MODULE=on CGO_ENABLED=0 GOOS=$(GOOS) GOARCH=$(GOARCH) GOPROXY=$(GOPROXY) go build \ | ||
| -trimpath \ | ||
| -ldflags="$(LDFLAGS)" \ | ||
| -o=../../bin/cloud-controller-manager-aws-tests-ext . |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether the Makefile switches recipes to bash.
grep -n '^SHELL' Makefile || echo "No SHELL override found; make will use its default shell."
sed -n '51,56p' MakefileRepository: openshift/cluster-cloud-controller-manager-operator
Length of output: 434
Use cd instead of pushd in this recipe.
make recipes run under /bin/sh unless SHELL is overridden. pushd is a Bash builtin and will fail in CI with pushd: not found.
Proposed fix
cloud-controller-manager-aws-tests-ext:
- pushd cmd/cloud-controller-manager-aws-tests-ext && \
+ cd cmd/cloud-controller-manager-aws-tests-ext && \
GO111MODULE=on CGO_ENABLED=0 GOOS=$(GOOS) GOARCH=$(GOARCH) GOPROXY=$(GOPROXY) go build \
-trimpath \
-ldflags="$(LDFLAGS)" \
-o=../../bin/cloud-controller-manager-aws-tests-ext .📝 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.
Suggested change
| cloud-controller-manager-aws-tests-ext: | |
| pushd cmd/cloud-controller-manager-aws-tests-ext && \ | |
| GO111MODULE=on CGO_ENABLED=0 GOOS=$(GOOS) GOARCH=$(GOARCH) GOPROXY=$(GOPROXY) go build \ | |
| -trimpath \ | |
| -ldflags="$(LDFLAGS)" \ | |
| -o=../../bin/cloud-controller-manager-aws-tests-ext . | |
| cloud-controller-manager-aws-tests-ext: | |
| cd cmd/cloud-controller-manager-aws-tests-ext && \ | |
| GO111MODULE=on CGO_ENABLED=0 GOOS=$(GOOS) GOARCH=$(GOARCH) GOPROXY=$(GOPROXY) go build \ | |
| -trimpath \ | |
| -ldflags="$(LDFLAGS)" \ | |
| -o=../../bin/cloud-controller-manager-aws-tests-ext . |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Makefile` around lines 51 - 56, The Makefile recipe for target
cloud-controller-manager-aws-tests-ext uses the Bash builtin pushd which fails
under /bin/sh in CI; replace the pushd/popd usage with a POSIX-safe change
directory by using cd into cmd/cloud-controller-manager-aws-tests-ext (or wrap
the build in a subshell with (cd cmd/cloud-controller-manager-aws-tests-ext &&
GO111MODULE=... go build ...)) so the build runs under /bin/sh without requiring
Bash.
Contributor
Author
|
/testwith openshift/cluster-cloud-controller-manager-operator/main/e2e-aws-ovn openshift/origin#30864 openshift/cloud-provider-aws#133 |
mtulio
added a commit
to mtulio/openshift-origin
that referenced
this pull request
Mar 12, 2026
Removing CCM-AWS OTE binary extraction temporarialy to unblock upstream sync. This will be migrated to CCCMO project to prevent future issues and keep the OTE extractions. See also: openshift/cluster-cloud-controller-manager-operator#437
Contributor
Author
|
I don't know why multi job keeps failing, the recent changes fixed the cache? I will retry: |
Contributor
Author
|
/testwith openshift/cluster-cloud-controller-manager-operator/main/e2e-aws-ovn openshift/origin#30864 openshift/cloud-provider-aws#133 |
2 similar comments
Contributor
Author
|
/testwith openshift/cluster-cloud-controller-manager-operator/main/e2e-aws-ovn openshift/origin#30864 openshift/cloud-provider-aws#133 |
Contributor
Author
|
/testwith openshift/cluster-cloud-controller-manager-operator/main/e2e-aws-ovn openshift/origin#30864 openshift/cloud-provider-aws#133 |
Contributor
Author
|
Following this job, I can see the binary The job is still running. |
|
@mtulio: This pull request references SPLAT-2672 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 story to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
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. |
Contributor
Author
|
/testwith openshift/cluster-cloud-controller-manager-operator/main/e2e-aws-ovn openshift/origin#30864 openshift/cloud-provider-aws#133 |
Contributor
Author
|
origin PR rebase to temp removing CCM-AWS OTE to unblock cloud team, re-testing: |
Contributor
Author
|
/testwith openshift/cluster-cloud-controller-manager-operator/main/e2e-aws-ovn openshift/origin#30864 openshift/cloud-provider-aws#133 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Blocks:
Summary by CodeRabbit
New Features
Documentation
Chores