Skip to content

Add restart controller#11

Merged
sbernauer merged 8 commits intomainfrom
feature/restart-controller-2
Apr 14, 2022
Merged

Add restart controller#11
sbernauer merged 8 commits intomainfrom
feature/restart-controller-2

Conversation

@sbernauer
Copy link
Copy Markdown
Member

@sbernauer sbernauer commented Mar 30, 2022

Description

For stackabletech/issues#200
Actual implementation copied from https://github.com/stackabletech/restart-controller
Integration-Test: https://github.com/stackabletech/integration-tests/pull/207

Review Checklist

  • Code contains useful comments
  • (Integration-)Test cases added (or not applicable)
  • Documentation added (or not applicable)
  • Changelog updated (or not applicable)
  • Cargo.toml only contains references to git tags (not specific commits or branches)
  • Helm chart can be installed and deployed operator works (or not applicable)

Once the review is done, comment bors r+ (or bors merge) to merge. Further information

@sbernauer sbernauer requested review from a team and nightkr March 30, 2022 11:07
Copy link
Copy Markdown
Member

@maltesander maltesander left a comment

Choose a reason for hiding this comment

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

Just some comments. Probably @teozkr needs to have another look :)

Comment thread deploy/helm/commons-operator/templates/roles.yaml Outdated
Comment thread deploy/manifests/roles.yaml Outdated
Comment thread rust/operator-binary/src/restart_controller.rs Outdated
Comment thread rust/operator-binary/src/restart_controller.rs
Comment thread rust/operator-binary/src/restart_controller.rs Outdated
Copy link
Copy Markdown
Contributor

@nightkr nightkr left a comment

Choose a reason for hiding this comment

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

LGTM for now, there's some documentation missing and generalization that we might want to do, but that's more of a problem that I need to get back to myself...

@sbernauer
Copy link
Copy Markdown
Member Author

Thanks! Actually I intended to write some docs after this is merged. But for the generalization i think you're the best man!

@sbernauer sbernauer merged commit 1e84efa into main Apr 14, 2022
@bors bors Bot deleted the feature/restart-controller-2 branch April 14, 2022 06:19
@lfrancke
Copy link
Copy Markdown
Member

Do we have a ticket for the documentation @sbernauer? If not, could you create one?

@sbernauer
Copy link
Copy Markdown
Member Author

There is no dedicated ticket for docs but the docs are tracked as part of stackabletech/issues#200

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.

4 participants