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

Enable reconcile block via CR annotation #861

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

lishanley
Copy link

@lishanley lishanley commented Apr 9, 2024

Hi,

This PR is in reference to the issue raised here - #685

This PR provides the operator manager the ability to block reconciliation of brokers via a CR annotation. Steps on how to make use of this functionality can be found below:

Running the following command will block reconciliation:

kubectl annotate activemqartemises.broker.amq.io artemis-broker artemis.broker.amq.io/block-reconcile='true'

When this annotation is present on the CR the operator manager pod will log the following message when changes are being made to the CR while reconciliation is blocked:

INFO ActiveMQArtemisReconciler Reconcile blocked by annotation {"Request.Namespace": "activemq-artemis-operator", "Request.Name": "artemis-broker", "Reconciling": "ActiveMQArtemis"}

To unblock reconciliation run the following command to set the flag on the annotation to false:

kubectl annotate --overwrite activemqartemises.broker.amq.io artemis-broker artemis.broker.amq.io/block-reconcile='false'.

This functionality has been tested using the main branch and minikube.

I am happy to take any feedback and suggestions to help improve this solution if required.

@gtully
Copy link
Contributor

gtully commented Apr 10, 2024

I think it needs a test, peek at one of the validation tests for inspiration. Which leads to the question, should the presence of the annotation be reflected in a status condition like ReconcileBlocked. The Status is the first port of call for users who want to know the state of their CR. I think it would be a good addition.
Here is a test that can be a good start point: https://github.com/artemiscloud/activemq-artemis-operator/blob/main/controllers/activemqartemis_controller_test.go#L831

@lishanley
Copy link
Author

lishanley commented Apr 11, 2024

Hi @gtully - Thank you for your feedback, it is much appreciated.

Sure, I can work on putting a validation test together for this contribution. I can also work on putting together a status condition. Once complete, I will re commit.

I can see that one of the checks failed due to the latest commit message. When re committing i'll also ensure the correct format is implemented.

Thanks!

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.

None yet

2 participants