-
Notifications
You must be signed in to change notification settings - Fork 21
test: refactor custom install suite and add rolling update scenario #1292
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Reviewer's GuideThis PR refactors the custom install E2E suite to support flexible manager pod options (NFS-backed PVCs and dynamic image injection), adds new HTTP proxy and rolling-upgrade test scenarios, and updates CI workflows to install and cleanup NFS CSI support. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
@sourcery-ai title |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes - here's some feedback:
- Replace the static time.Sleep calls used to wait for operator pod deletion with Gomega Eventually loops checking pod readiness for more reliable test timing.
- Extract the installOperator, managerPod, and rbac helper functions into shared test utilities to reduce duplicated setup code across the proxy and rolling upgrade tests.
- Consider using kustomize or the controller-runtime typed client directly instead of unstructured YAML decoding when applying RBAC resources for stronger type safety and maintainability.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Replace the static time.Sleep calls used to wait for operator pod deletion with Gomega Eventually loops checking pod readiness for more reliable test timing.
- Extract the installOperator, managerPod, and rbac helper functions into shared test utilities to reduce duplicated setup code across the proxy and rolling upgrade tests.
- Consider using kustomize or the controller-runtime typed client directly instead of unstructured YAML decoding when applying RBAC resources for stronger type safety and maintainability.
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
a4f0a5d
to
1ca1686
Compare
1ca1686
to
9e2dcd1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `.github/actions/kind-cluster/action.yml:118` </location>
<code_context>
echo "oidc_host=keycloak-internal.keycloak-system.svc" >> $GITHUB_OUTPUT
+
+ - name: Install nfs-csi
+ if: ${{ inputs.nfs-csi == 'true'}}
+ id: install-nfs-csi
+ shell: bash
+ run: |
+ kustomize build --enable-helm ./ci/nfs/overlay/ | kubectl apply -f -
\ No newline at end of file
</code_context>
<issue_to_address>
No error handling for kustomize or kubectl failures.
Add error handling to ensure failures in kustomize or kubectl are clearly reported, such as using set -e or explicit checks.
</issue_to_address>
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
d9c1a43
to
5755535
Compare
5755535
to
041ff90
Compare
Refactor custom install suite to allow install different configurations of manager pod. Create a new scenario to trigger rolling update on all deployments with multiple replicas. Signed-off-by: Tomas Turek <[email protected]>
041ff90
to
ef50e8f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `.github/actions/kind-cluster/action.yml:118` </location>
<code_context>
echo "oidc_host=keycloak-internal.keycloak-system.svc" >> $GITHUB_OUTPUT
+
+ - name: Install NFS-CSI
+ if: ${{ inputs.nfs-csi == 'true'}}
+ id: install-nfs-csi
</code_context>
<issue_to_address>
Consider adding error handling for NFS-CSI installation.
Without error handling, failures in kustomize or kubectl may go unnoticed, resulting in an incomplete NFS-CSI setup. Implement checks to ensure installation succeeds.
</issue_to_address>
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Refactor custom install suite to allow install different configurations of manager pod.
Create a new scenario to trigger rolling update on all deployments with multiple replicas.
Summary by Sourcery
Refactor custom install suite, introduce NFS-CSI support, and add new e2e custom_install tests for proxy-based installs and rolling upgrade scenarios, with corresponding CI workflow updates
New Features:
Enhancements:
CI:
Tests: