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

DBZ-8025: First Pass at adding Observed Generation #67

Merged
merged 1 commit into from
Jul 16, 2024

Conversation

ryanvanhuuksloot
Copy link
Contributor

Attempting a first pass at adding observed generation to the status.observedGeneration spec.

I couldn't figure out how to test this though. I built and loaded the jar(s) into my operator but it wouldn't pick up the latest code - not even log lines I tested for debugging.

I also realize that I need to add tests - but since more than just Conditions now are changing in status, I think it is worth a discussion on how the status should be handled.

@jcechace
Copy link
Member

jcechace commented Jul 8, 2024

LGTM

Regarding testing: this is also something we need to work on. Technically right now tests in the core module are set to only use api server. I intend to switch that to full kind and have some base tests there.

As for building -- I think a contributor guide should help. Since we are releasing also helm charts and OLM bundles the process is a bit complex -- while the operator is technically just the core module, the final build is driven by the dist module.

To make things worse, I probably have a bit incorrect quarkus setup -- which results in devmode having troubles picking up code changes in other modules than core. I intend to fix that as well for 3.0 after consulting a friend.

@ryanvanhuuksloot
Copy link
Contributor Author

Sounds good - no rush on merging this - we aren't blocked right now. I'm just looking at removing future blockers early.

Happy to wait for a contributor guide and be the first drive it.

@jcechace
Copy link
Member

@ryanvanhuuksloot if you consider this done I'm happy to merge this.

@ryanvanhuuksloot
Copy link
Contributor Author

I want to test it fully in our working environment first. I think something is missing/misconfigured because it doesn't work as I thought it would in my testing environment.
I applied the new CRD, a new operator image with the new core and api jar. Yet seems to fail writing the status. It doesn't look like a permission error.

I was hoping for some guide to deploy operator changes to validate that it is the code and not how I deployed the new operator code.

@ryanvanhuuksloot ryanvanhuuksloot marked this pull request as ready for review July 12, 2024 20:22
@jcechace jcechace merged commit 0fd69c1 into debezium:main Jul 16, 2024
3 checks passed
@jcechace
Copy link
Member

I did some preliminary testing and it seems to work fine. Thanks for the contribution!

@ryanvanhuuksloot ryanvanhuuksloot deleted the DBZ-8025 branch July 16, 2024 12:40
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