-
Notifications
You must be signed in to change notification settings - Fork 44
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
microcloud charm v0.1 #149
microcloud charm v0.1 #149
Conversation
Signed-off-by: Gabriel Mougard <[email protected]>
…th JuJu Signed-off-by: Gabriel Mougard <[email protected]>
…erations Signed-off-by: Gabriel Mougard <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, LGTM! I quite like the nginx cache trick you pulled to make testing faster, love it!
check=True, | ||
timeout=600, | ||
) | ||
self.unit_maintenance("core core20 core22 installed successfully...") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't deploy any of the core
. The needed ones are pulled automatically as snapd
handles dependencies.
config_changed = self.config_changed() | ||
logger.info(f"Validating config: {config_changed}") | ||
|
||
# TODO: For now, we don't have any config to validate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enabling/disabling microovn
/microceph
on an already deployed app should probably be rejected.
self.microcloud_remove(os.uname().nodename) | ||
logger.info("Microcloud node successfully removed") | ||
except RuntimeError: | ||
logger.error("Failed to remove a Microcloud node, retrying later") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'd do it the other way around. Instead of the removed unit trying to self remove, why not ask the leader to remove the departing unit from the cluster?
I'm suggesting that as rel-broken fires very late and you often don't have access to data bags.
if not self.unit.is_leader(): | ||
time.sleep(30) # Make sure that the leader has time to initialize the cluster | ||
|
||
return |
There was a problem hiding this 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 delaying the return achieves anything, or does it? Isn't the leader supposed to bootstrap the cluster and then only add nodes?
@gabrielmougard we will not be merging this PR into this repo, but will instead create a new repo called charm-microcloud. I am currently waiting for confirmation on what license to use and will let you know when the new repo is setup. In the meantime it'd be worth addressing @simondeziel comments here so that your branch is ready to be sent to the new repo once its created. |
@gabrielmougard new repo is created here https://github.com/canonical/charm-microcloud |
This is the initial work for the Microcloud MAAS charm.
As of now the charm achieves the following:
microceph
andmicroovn
.--auto
flag.Roadmap items to achieve for 0.2:
Roadmap items to achieve for 1.0:
preseed
bootstrapping #154scenario
#155