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

Make PodDisruptionBudget an optional feature #540

Merged
merged 9 commits into from
Apr 14, 2023

Conversation

HoustonPutman
Copy link
Contributor

@HoustonPutman HoustonPutman commented Mar 31, 2023

resolves #538

List to do:

  • Create CRD Options
  • Implement feature in controllers
  • Add unit tests
  • Add Helm values and docs
  • Implement Helm values in SolrCloud creation
  • Add docs
  • Add changelog

@HoustonPutman HoustonPutman added the good first issue Good for newcomers label Mar 31, 2023
@HoustonPutman HoustonPutman changed the title Add CRD options, helm options and a changelog entry Make PodDisruptionBudget an optional feature Apr 5, 2023
@gerlowskija gerlowskija self-assigned this Apr 7, 2023
@gerlowskija
Copy link
Contributor

Taking a look at this now as a way to brush up on operator code. Thanks for all the work you put into documenting this Houston!

This commit adds the necessary logic during solrcloud reconciliation to
obey the enable/disable flag for PDB creation.A

Still needed is integration with the solrcloud helm chart.
@gerlowskija
Copy link
Contributor

Alright, should be ready for review. Lmk if I missed anything!

Copy link
Contributor Author

@HoustonPutman HoustonPutman left a comment

Choose a reason for hiding this comment

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

Looks great!

A few hopefully quick changes, but overall great!

api/v1beta1/solrcloud_types.go Outdated Show resolved Hide resolved
controllers/solrcloud_controller_test.go Show resolved Hide resolved
controllers/solrcloud_controller_test.go Outdated Show resolved Hide resolved
docs/solr-cloud/solr-cloud-crd.md Outdated Show resolved Hide resolved
helm/solr/templates/solrcloud.yaml Show resolved Hide resolved
return requeueOrNot, err
}
} else { // PDB is disabled, make sure that we delete any previously created pdb that might exist.
err = r.Client.Delete(ctx, pdb)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interesting... So it's basically as good to delete something that might not exist as it is to check if it exists first before deleting it.

Copy link
Contributor Author

@HoustonPutman HoustonPutman left a comment

Choose a reason for hiding this comment

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

Looks good to me!!

@gerlowskija gerlowskija merged commit ca80a1a into apache:main Apr 14, 2023
@HoustonPutman HoustonPutman deleted the pdb-options branch April 21, 2023 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make PodDisruptionBudget an optional feature
2 participants