feat: adding flavor bootstrap for nova #290
Conversation
intlabs
left a comment
intlabs
left a comment
There was a problem hiding this comment.
Looks good so far @larryrensing :) just a few comments.
nova/values.yaml
Outdated
| service: | ||
| - mariadb | ||
| bootstrap: | ||
| jobs: |
There was a problem hiding this comment.
Does the bootstrap job need to be dependent on any jobs itself? I think these can most likely all be safely removed, as it is only really dependent on the nova & keystone api's.
nova/values.yaml
Outdated
| - keystone-db-init | ||
| - mariadb-seed | ||
| service: | ||
| - mariadb |
There was a problem hiding this comment.
It should be safe to remove this dependency - as the bootstrap job will only be dependent on the api's themselves, and not the db backing them.
nova/values.yaml
Outdated
| enabled: true | ||
| flavors: | ||
| m1: | ||
| tiny: |
There was a problem hiding this comment.
it would probably be better to have this as a single key, eg m1_tiny rather than a nest.
There was a problem hiding this comment.
+1 would like see this as well. and this should really be something where company "acme" could add a list of flavors, no? even personally, i always have my 20 or so flavors after every fresh installation.
nova/templates/bin/_bootstrap.sh.tpl
Outdated
|
|
||
| wait_for_api | ||
|
|
||
| nova flavor-show {{ .Values.bootstrap.flavors.m1.tiny.name }} || \ |
There was a problem hiding this comment.
would it be possible to expand this to an arbitrary number of flavors, specified in the values.yaml?
nova/templates/bin/_bootstrap.sh.tpl
Outdated
| TIMEOUT=120 | ||
| #larryrensing: sleep to wait until nova api is actually serving on | ||
| # the appropriate port. not pretty, there has to be a cleaner approach to this | ||
| sleep 30 |
There was a problem hiding this comment.
Moving to direct service checking along the lines of: #143 would remove this limitation of kubernetes-entrypoint to determine if a service is ready to receive requests. Until then is the sleep and the while statement needed or could we reduce it to one? If the job initially fails it presumably restarts wich may be a simpler, yet less pretty, way to deal with this.
nova/values.yaml
Outdated
| enabled: true | ||
| flavors: | ||
| m1: | ||
| tiny: |
There was a problem hiding this comment.
+1 would like see this as well. and this should really be something where company "acme" could add a list of flavors, no? even personally, i always have my 20 or so flavors after every fresh installation.
nova/templates/bin/_bootstrap.sh.tpl
Outdated
| function wait_for_api { | ||
| TIMEOUT=120 | ||
| #larryrensing: sleep to wait until nova api is actually serving on | ||
| # the appropriate port. not pretty, there has to be a cleaner approach to this |
There was a problem hiding this comment.
were you planning on removing this statement, or just your github handle? i'd really like to remove the personal references (notes are fine). trying to catch them when we see them.
larryrensing
force-pushed
the
feat/bootstrap-services-nova
branch
from
54e8c03 to
3cd21f9
Compare
nova/templates/job-bootstrap.yaml
Outdated
| - name: nova-bin | ||
| configMap: | ||
| name: nova-bin | ||
| {{- end -}} |
There was a problem hiding this comment.
See #294 for comments on whether the job is conditional or the work is conditional.
nova/templates/bin/_bootstrap.sh.tpl
Outdated
| export HOME=/tmp | ||
|
|
||
|
|
||
| function wait_for_api { |
There was a problem hiding this comment.
Why aren't we guaranteed this with the dependency checks? If we're not, we may want to investigate there.
nova/templates/bin/_bootstrap.sh.tpl
Outdated
| } | ||
|
|
||
| cat <<EOF>/tmp/openrc | ||
| export OS_USERNAME={{.Values.keystone.admin_user}} |
There was a problem hiding this comment.
Isn't there a helm-toolkit macro for this?
There was a problem hiding this comment.
Yes - it was merged after this PR was submitted, a rebase is needed
larryrensing
force-pushed
the
feat/bootstrap-services-nova
branch
from
79ae872 to
3d82041
Compare
alanmeadows
left a comment
alanmeadows
left a comment
There was a problem hiding this comment.
Looks good Larry. We can continue to enhance this.
4 participants
What is the purpose of this pull request?:
This gives users an option to load nova with a basic flavor.
What issue does this pull request address?: partial fix for #201
Notes for reviewers to consider:
The setting of env vars in bootstrap.sh will change when #273 is merged
Specific reviewers for pull request:
@intlabs @alanmeadows