-
Notifications
You must be signed in to change notification settings - Fork 360
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
Add containerinfra nodegroup support #1364
Add containerinfra nodegroup support #1364
Conversation
8a1f783
to
0d5a60e
Compare
Build failed.
|
0d5a60e
to
70d77d5
Compare
Build failed.
|
70d77d5
to
5f3f4d4
Compare
Build failed.
|
277a23a
to
c99eea7
Compare
Build failed.
|
Build failed.
|
08226a9
to
322829e
Compare
Build failed.
|
@MrFreezeex ping, is there any progress on this? Thanks |
Well AFAIK it works just fine, but it won't probably be merged anytime soon because of the various pinned issue, including: #1366 |
3b50aef
to
8d96bba
Compare
Signed-off-by: Arthur Outhenin-Chalandre <arthur.outhenin-chalandre@cern.ch>
Signed-off-by: Arthur Outhenin-Chalandre <arthur.outhenin-chalandre@cern.ch>
Signed-off-by: Arthur Outhenin-Chalandre <arthur.outhenin-chalandre@cern.ch>
Signed-off-by: Arthur Outhenin-Chalandre <arthur.outhenin-chalandre@cern.ch>
Signed-off-by: Arthur Outhenin-Chalandre <arthur.outhenin-chalandre@cern.ch>
8d96bba
to
0106f75
Compare
I also fixed the merge_labels behavior of the cluster resource/added merge_labels support for nodegroup since this gophercloud/gophercloud#2377 was merged and released in 0.25.0. |
0106f75
to
773483b
Compare
When you add mergelabels and you actually add labels, the magnum API will actually return you the complete set of labels under the "labels" key. It means that if you want to add/override a couple of label while setting merge_labels the creation will succeed but then terraform will detect the rest of the labels as changed and will try to rebuid the cluster. This commit instead use the keys labels_{added,skipped,overriden} when merge labels which contains the labels that the user has overriden. Signed-off-by: Arthur Outhenin-Chalandre <arthur.outhenin-chalandre@cern.ch>
Signed-off-by: Arthur Outhenin-Chalandre <arthur.outhenin-chalandre@cern.ch>
773483b
to
d908d53
Compare
Signed-off-by: Arthur Outhenin-Chalandre <arthur.outhenin-chalandre@cern.ch>
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.
LGTM.
I can merge this without running acceptance tests in CI if it's ready and has been manually tested.
Oh ok, thanks! Hmm I didn't look into running the acceptance tests on our openstack cluster so far, let me check how I can do that! |
44b7da7
to
9687439
Compare
Signed-off-by: Arthur Outhenin-Chalandre <arthur.outhenin-chalandre@cern.ch>
Previously the code set image_id and flavor_id which doesn't exist instead of image/flavor. It would be way more meaningful to have argument image_id and flavor_id instead but for backward compatibility let's not do that for now. Signed-off-by: Arthur Outhenin-Chalandre <arthur.outhenin-chalandre@cern.ch>
As fedora atomic is no more available, we remove it from the code and add OS_MAGNUM_IMAGE environment variable so that we can a supported image in the CI later on. Signed-off-by: Arthur Outhenin-Chalandre <arthur.outhenin-chalandre@cern.ch>
The node_count returned in magnum for the cluster is the sum of all nodegroup. So once you add a node_count, the correct value is actually the node_count of the default worker node. This commit take the node_count of this default worker node group in priority and if there are any errors it takes the node_count of the cluster instead. Signed-off-by: Arthur Outhenin-Chalandre <arthur.outhenin-chalandre@cern.ch>
Signed-off-by: Arthur Outhenin-Chalandre <arthur.outhenin-chalandre@cern.ch>
9687439
to
8d1e619
Compare
@ozerovandrei I tested the acceptance tests on our OpenStack cluster and it now works. I had to make a couple of changes including changing the way we get the magnum image as the one that we used was deprecated since a long time and the link was not available anymore. I suspect that the acceptance was not working on the cluster resource also as I fixed a bug there, now everything should be working properly! |
Thank you @MrFreezeex |
This PR adds resource and data source support for OpenStack Magnum node groups.
I added support for zero node_count cluster and nodegroup.
And I also fixed the merge_labels behavior and added it to the nodegroup resource as well