sureshanaparti commented on PR #105:
URL:
https://github.com/apache/cloudstack-kubernetes-provider/pull/105#issuecomment-5617074691
@vishesh92 can you check the below.
- Duplicate-rule cleanup assumes same public IP (please confirm or guard)
In _getLoadBalancer_, a name-collided rule hits continue before the
_lb.ipAddr_ consistency check, and _deleteDuplicateRules_ only deletes the LB
rule. If a duplicate ever had a different public IP, its firewall rule / IP
would leak on cleanup — the exact class of bug this PR fixes elsewhere. Can we
either add a guard, or a one-line comment confirming duplicates are guaranteed
to share IP/port/protocol?
- Guard the e2e job like the build job
build is gated on _if: github.repository == 'apache/...' ||
workflow_dispatc_h, but e2e (needs: build) has no matching if. A
workflow_dispatch from a fork (or a repo rename) would leave e2e running with
no artifact to download. Also cache-to: type=gha,mode=max is read-only on fork
PRs and will warn. Suggest mirroring the if: onto e2e.
- make test now runs against the live simulator in CI
Sourcing _keys.env_ before make test activates the acceptance tests in
_cloudstack_test.go_ against the simulator. Intentional and useful, but it
couples the unit-test signal to simulator health. Worth confirming that's the
intent — or splitting the pure-unit run from the live run so a sim hiccup
doesn't read as a code failure.
- Minor nits (can batch into one comment)
- _.gitignore_ is missing a trailing newline.
- _retry()_ in hack/e2e/lib/log.sh looks unused — drop it?
- _docs/development.md_ says "Go 1.23 or later" while CI uses
go-version-file: go.mod — keep in sync with go.mod.
- _majorMinorPatch_ is duplicated in cloudstack.go and
test/e2e/framework.go (unavoidable across the package boundary) — worth a note
so a future edit updates both.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
