Add containerinfra nodegroup support #1364
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MrFreezeex
force-pushed
the
magnum-nodegroup
branch
from
8a1f783
to
0d5a60e
Compare
|
Build failed.
|
MrFreezeex
force-pushed
the
magnum-nodegroup
branch
from
0d5a60e
to
70d77d5
Compare
|
Build failed.
|
MrFreezeex
force-pushed
the
magnum-nodegroup
branch
from
70d77d5
to
5f3f4d4
Compare
|
Build failed.
|
MrFreezeex
force-pushed
the
magnum-nodegroup
branch
8 times, most recently
from
277a23a
to
c99eea7
Compare
|
Build failed.
|
|
Build failed.
|
MrFreezeex
force-pushed
the
magnum-nodegroup
branch
from
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 |
MrFreezeex
force-pushed
the
magnum-nodegroup
branch
from
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>
MrFreezeex
force-pushed
the
magnum-nodegroup
branch
from
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. |
MrFreezeex
force-pushed
the
magnum-nodegroup
branch
from
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>
MrFreezeex
force-pushed
the
magnum-nodegroup
branch
from
773483b
to
d908d53
Compare
Signed-off-by: Arthur Outhenin-Chalandre <arthur.outhenin-chalandre@cern.ch>
ozerovandrei
approved these changes
Jun 21, 2022
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! |
MrFreezeex
force-pushed
the
magnum-nodegroup
branch
3 times, most recently
from
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>
MrFreezeex
force-pushed
the
magnum-nodegroup
branch
from
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 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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