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

MaaS provider should expose the bootstrap timeout options. #18

Merged
merged 3 commits into from Jun 13, 2014
Merged

MaaS provider should expose the bootstrap timeout options. #18

merged 3 commits into from Jun 13, 2014

Conversation

niedbalski
Copy link
Contributor

Most of the MAAS deployments takes longer times, in fact a install with d-i typically takes around 20 minutes on a good day.

Would be a good thing to at least have the timeout settings exposed ( commented ) for being
noticed by all users.

Please see reference LP: #1325891

@mjs
Copy link

mjs commented Jun 4, 2014

LGTM although I wonder if we should also change the default for MAAS if 600 is never going to be enough?

# bootstrap-timeout: 600

# bootstrap-retry-delay time between attempts to connect to an address, in seconds.
# bootstrap-retry-delay: 5
Copy link

Choose a reason for hiding this comment

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

If it's just the bootstrap that's slow (and that's addressed by bootstrap-timeout) why are bootstrap-retry-delay and bootstrap-addresses-delay also included? Is there something special about MAAS that means it makes sense to also show bootstrap-retry-delay and bootstrap-addresses-delay here?

@niedbalski
Copy link
Contributor Author

Hey @mjs ,

In fact, this MR is just a cosmetic change, the real change would be to double ( 1200 sec ) the bootstrap-timeout.

@jameinel
Copy link
Member

jameinel commented Jun 5, 2014

Well, if 20min is a good day, then it sounds like we actually need to set
it to at least 1800 (30min).

I think it would be reasonable to only expose bootstrap-timeout by default,
and have it uncommented at 1800 for MaaS. (so by default 'juju init' gives
you something that has a long enough timeout that we expect it to JustWork.)

John
=:->

On Thu, Jun 5, 2014 at 5:23 AM, Jorge Niedbalski notifications@github.com
wrote:

Hey @mjs https://github.com/mjs ,

In fact, this MR is just a cosmetic change, the real change would be to
double ( 1200 sec ) the bootstrap-timeout.


Reply to this email directly or view it on GitHub
#18 (comment).

@fwereade
Copy link
Contributor

fwereade commented Jun 5, 2014

I agree we should expose the timeout setting for all providers; but shouldn't we just have the maas provider set a longer default if there's nothing specified in the raw data?

Updated bootstrap-timeout to 1800 secs by default on MAAS provider.
@niedbalski
Copy link
Contributor Author

@jameinel , i leave the bootstrap-timeout: 1800 by default.

Thanks.

@mjs
Copy link

mjs commented Jun 6, 2014

@fwereade isn't just including a higher value in the default configuration file almost as good as encoding a higher default in the provider implementation?

@fwereade
Copy link
Contributor

fwereade commented Jun 9, 2014

@mjs almost as good, yeah, but it's still just one more thing to remember (and type) when adding an env from scratch; not to mention one more thing to read when looking at an environment config. And all this is in service of something that should work anyway.

Also ISTM (given that it's globally applicable) there'd be something subtly misleading about putting it in the sample maas config but not elsewhere... and putting it everywhere definitely seems like celebrating it beyond what it deserves.

@niedbalski
Copy link
Contributor Author

Hey @mjs , @fwereade

Leaving this variable exposed on the configuration boilerplate with the 1800 secs value fixes my initial issue.

The question about what a good default value should be set, which is provider-dependent goes one step further on the same direction.

How we should proceed? Suggestions?

@fwereade
Copy link
Contributor

  • unless you're driving specific value from them, drop the bootstrap-retry-delay, bootstrap-addresses-delay from the boilerplate config
  • otherwise, go ahead and merge as-is, because we don't have any provider-specific way to check the attrs before they get parsed into a *Config.
  • please do not mark the LP bug fixed (because the scope of juju init is somewhat limited, and not everybody will see the benefits of this change); or, if you do (because after all it does address your issue), please open a new one complaining that (i) the bootstrap timeout default is bad for maas, and that can't easily be fixed properly because (ii) there's no way for a specific provider to choose defaults for *Config-level values.

Just bootstrap-timeout option will be exposed, until this default value is increased
on a per-provider basis.
@niedbalski
Copy link
Contributor Author

Hello @fwereade ,

I modified this pull request to just expose the boostrap-timeout option in the boilerplate file. I pointed your comment on the public launchpad bug (https://bugs.launchpad.net/juju-core/+bug/1314665) moving that to 'Confirmed' , waiting for a juju-core team solution for having provider specific chosen configuration defaults.

Thanks.

@mjs
Copy link

mjs commented Jun 11, 2014

Thanks Jorge! LGTM.

@axw
Copy link
Contributor

axw commented Jun 13, 2014

$$merge$$

@jujubot
Copy link
Collaborator

jujubot commented Jun 13, 2014

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

@jujubot
Copy link
Collaborator

jujubot commented Jun 13, 2014

Build failed: Merging failed
build url: http://juju-ci.vapour.ws:8080/job/github-merge-juju/127

@axw
Copy link
Contributor

axw commented Jun 13, 2014

$$merge$$

@jujubot
Copy link
Collaborator

jujubot commented Jun 13, 2014

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

@jujubot
Copy link
Collaborator

jujubot commented Jun 13, 2014

Build failed: Merging failed
build url: http://juju-ci.vapour.ws:8080/job/github-merge-juju/129

jujubot added a commit that referenced this pull request Jun 13, 2014
MaaS provider should expose the bootstrap timeout options.

Most of the MAAS deployments takes longer times, in fact a install with d-i typically takes around 20 minutes on a good day.

Would be a good thing to at least have the timeout settings exposed ( commented ) for being
noticed by all users.

Please see reference LP: #1325891
@jujubot jujubot merged commit ead2e2d into juju:master Jun 13, 2014
anastasiamac pushed a commit to anastasiamac/juju that referenced this pull request Jan 29, 2015
Rootfs provider/pool, various improvements
ericsnowcurrently pushed a commit to ericsnowcurrently/juju that referenced this pull request May 26, 2015
4a6f656c pushed a commit to 4a6f656c/juju that referenced this pull request Apr 20, 2017
Provide a basic 'juju status' implementation for CAAS models
wupeka pushed a commit to wupeka/juju that referenced this pull request Dec 17, 2017
get-bundle-changes: print all errors when the given bundle is not valid.
hmlanigan pushed a commit to hmlanigan/juju that referenced this pull request Aug 26, 2019
Add the method to start machines

Based on PRs juju#16 and juju#17.
laszlokajtar pushed a commit to laszlokajtar/juju that referenced this pull request Oct 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants