Skip to content

CLOUDP-328219: Flex to Dedicated reconciliation #2526

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

Merged
merged 3 commits into from
Jul 18, 2025

Conversation

helderjs
Copy link
Collaborator

Summary

Proof of Work

Checklist

  • Have you linked a jira ticket and/or is the ticket in the title?
  • Have you checked whether your jira ticket required DOCSP changes?
  • Have you checked for release_note changes?
  • Have you signed our CLA?

Reminder (Please remove this when merging)

  • Please try to Approve or Reject Changes the PR, keep PRs in review as short as possible
  • Remember the following Communication Standards - use comment prefixes for clarity:
    • blocking: Must be addressed before approval.
    • follow-up: Can be addressed in a later PR or ticket.
    • q: Clarifying question.
    • nit: Non-blocking suggestions.
    • note: Side-note, non-actionable. Example: Praise
    • --> no prefix is considered a question

@helderjs helderjs requested a review from a team as a code owner July 17, 2025 10:24
@helderjs helderjs force-pushed the CLOUDP-328219-reconciliation branch 3 times, most recently from a54aa9c to 9787b23 Compare July 17, 2025 10:56
@helderjs
Copy link
Collaborator Author

My 2 cents on the test approach:

  • This is basically an opt-out approach.
  • My proposal is the opposite opt-in.

The difference is that, yes, we do a bit more work up front with opt-in, but because it is opt-in we force ourselves to pick and chose exactly what we want to bring forward to the codebase of the future and what we want to leave behind.

The opt-out approach does the opposite, it feels less work initially, at the expense to blurs the lines between the code we really are still using and want to bring forward and the code that is left unused. This specially happens when reusing a library that mixes a lot of sometimes more and sometimes less related facilities. This lack of clarity means we might never do the cleanup at the end. In my mind, this is more important specially if the cleanup cannot happen before 2 or 3 years from now.

To make it clear from the very beginning, I'm not reproducing fully what you have done, beside not using the DataProvider struct. I did the bare minimum to show it's possible to run the operator in a separate process and use yamls files, which I think are the big values of the new suite. All the rest are implementation details, code get improved all the time.
I also replicate some Ginkgo "mistakes" which are not idiomatic, like ordered tests where there are no dependency between them. Or async assertion done with logic instead of "assertion".

Copy link
Collaborator

@josvazg josvazg left a comment

Choose a reason for hiding this comment

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

LGTM as we are refactoring the e2e testing in a follow up

@helderjs helderjs merged commit 3757645 into main Jul 18, 2025
194 of 197 checks passed
@helderjs helderjs deleted the CLOUDP-328219-reconciliation branch July 18, 2025 10:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants