Skip to content
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

constellation-node-operator: don't bail out on listing errors #3522

Merged
merged 1 commit into from
Dec 3, 2024

Conversation

msanft
Copy link
Contributor

@msanft msanft commented Dec 3, 2024

Context

If the GCP project has scaling groups for which our checks can't be performed (which is the case for regional scaling groups, as they "don't exist" for the operator, if deployed in another region) . In that case, we should not bail out directly but go on with the next group. An error should only be thrown if there are no matching groups at all.

Proposed change(s)

  • Don't error directly, but only if no scaling groups are found.

Checklist

  • Run the E2E tests that are relevant to this PR's changes
  • Add labels (e.g., for changelog category)
  • Is PR title adequate for changelog?
  • Link to Milestone

@msanft msanft added the no changelog Change won't be listed in release changelog label Dec 3, 2024
@msanft msanft requested a review from 3u13r as a code owner December 3, 2024 08:30
Copy link

netlify bot commented Dec 3, 2024

Deploy Preview for constellation-docs ready!

Name Link
🔨 Latest commit c71defe
🔍 Latest deploy log https://app.netlify.com/sites/constellation-docs/deploys/674f2b61014f730008fe63bc
😎 Deploy Preview https://deploy-preview-3522--constellation-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@msanft msanft force-pushed the msanft/operator/gcp-scaling-groups branch from f649407 to 39117d1 Compare December 3, 2024 09:57
@msanft
Copy link
Contributor Author

msanft commented Dec 3, 2024

Another E2E

@msanft msanft force-pushed the msanft/operator/gcp-scaling-groups branch from 39117d1 to 188b474 Compare December 3, 2024 10:08
@msanft msanft force-pushed the msanft/operator/gcp-scaling-groups branch 2 times, most recently from 37ddceb to 1a1001d Compare December 3, 2024 10:09
@msanft msanft force-pushed the msanft/operator/gcp-scaling-groups branch from 1a1001d to 8776aab Compare December 3, 2024 10:13
Copy link
Member

@3u13r 3u13r left a comment

Choose a reason for hiding this comment

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

LGTM

@msanft msanft force-pushed the msanft/operator/gcp-scaling-groups branch from 8776aab to 71532b1 Compare December 3, 2024 15:41
If the GCP project has scaling groups for which our checks can't be performed (which is the case for regional scaling groups, as they "don't exist" for the operator, if deployed in another region) . In that case, we should not bail out directly but go on with the next group. An error should only be thrown if there are no matching groups at all.
@msanft msanft force-pushed the msanft/operator/gcp-scaling-groups branch from 71532b1 to c71defe Compare December 3, 2024 16:01
Copy link
Contributor

github-actions bot commented Dec 3, 2024

Coverage report

Package Old New Trend
operators/constellation-node-operator/internal/cloud/gcp/client 80.70% 80.20% ↘️

@msanft msanft merged commit b03e671 into main Dec 3, 2024
10 checks passed
@msanft msanft deleted the msanft/operator/gcp-scaling-groups branch December 3, 2024 21:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no changelog Change won't be listed in release changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants