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 all feature flags for new clusters #434

Merged
merged 2 commits into from
Nov 3, 2020
Merged

Conversation

ansd
Copy link
Member

@ansd ansd commented Oct 30, 2020

Closes #414

Notes:

  • The controller identifies a new RabbitMQ cluster by the operation result created of the StatefulSet.
  • A new annotation rabbitmq.com/createdAt is added onto the created StatefulSet.
  • Once all pods are ready, CLI command rabbitmqctl enable_feature_flag is used on the first pod of the StatefulSet to enable all stable and previously disabled feature flags. (In future, we could add feature flag methods to https://github.com/michaelklishin/rabbit-hole/ to not exec into the pod anymore.)

Copy link
Collaborator

@Zerpet Zerpet left a comment

Choose a reason for hiding this comment

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

I'm not sure if we should move the set plugins functionality to "post deploy", given that there are cases where the plugins might be updated without a (re)deploy. For example, by updating the CR with rabbitmq_federation, the plugin is enabled and the cluster should not be restarted.

Copy link
Contributor

@coro coro left a comment

Choose a reason for hiding this comment

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

Great changes, and thanks for the great refactoring along the way!

because the RabbitMQ CLI commands get executed at the end of the
reconcile loop which does not necessarily mean that a deployment just finished.

This commit also fixes a bug where the rabbitmq-plugins command was run on
every successful reconile loop.
@ansd
Copy link
Member Author

ansd commented Oct 30, 2020

@Zerpet good point, previously we ran "post deploy" at the end of every successful reconcile loop.
I added 63a5ae3 which renames the file and the function from "post deploy" to "CLI" because at the end of every reconcile loop we want to check whether there are any annotations which tell us that we need to run RabbitMQ CLI commands (such as setting plugins, rebalancing queues (which is a "post deploy" step), enabling feature flags).

The post deploy flag

SkipPostDeploySteps bool `json:"skipPostDeploySteps,omitempty"`
still exists and can be extended with other RabbitMQ CLI commands in future.

@ansd ansd requested a review from Zerpet October 30, 2020 18:22
@mkuratczyk
Copy link
Collaborator

mkuratczyk commented Nov 2, 2020

I think there is something wrong with the functionality.

It works if I just deploy-dev from the PR branch and create some clusters - feature flags get enabled.

However, I wanted to make sure it would not enable feature flags for pre-existing clusters so I deployed from main, created a cluster and then deployed from the branch and create another cluster. In that case feature flags were not enabled for either of the clusters - even a newly created cluster did not get them enabled.

@mkuratczyk
Copy link
Collaborator

Please ignore my comment above. Because we removed double prefix in main, I ended up with two operator running (the old one did the reconciliation)

Copy link
Collaborator

@Zerpet Zerpet left a comment

Choose a reason for hiding this comment

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

Latest changes look good :shipit:

@ansd ansd merged commit bab4c6e into main Nov 3, 2020
@ansd ansd deleted the enable-all-feature-flags branch November 3, 2020 10:00
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.

Enable all feature flags when deploying a new cluster
5 participants