-
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
Fix underlay configuration for preseed.yaml
#394
Conversation
23435d5
to
2f2af31
Compare
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! Only some minor comments but they can also be addressed later.
2f2af31
to
45eebbb
Compare
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, there is only one warning about usage of Println
instead of Printf
.
Signed-off-by: Max Asnaashari <[email protected]>
Signed-off-by: Max Asnaashari <[email protected]>
Signed-off-by: Max Asnaashari <[email protected]>
Signed-off-by: Max Asnaashari <[email protected]>
Signed-off-by: Max Asnaashari <[email protected]>
…ter subnet Signed-off-by: Max Asnaashari <[email protected]>
45eebbb
to
86b8b7a
Compare
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.
It looks good!
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.
Only two smaller comments, rest still looks good.
Looks like |
233aa59
to
86b8b7a
Compare
The logic for detecting the OVN underlay configuration for preseed was not correct, it was relying on
c.state
which contains the resources available and already set up on existing cluster members, as well as new cluster members. It is used in interactive setup to manage additional configuration being supplied to the existing systems.In preseed, we don't populate it yet because we only want to apply the precise recipe that is defined in the preseed, and not change any other cluster information implicitly (in interactive setup we can ask the user).
Additionally, the input parsing was not correct, since it expected a CIDR per system however the ovn encap configuration wants an IP.
So this addresses that by instead using the list of systems parsed from the yaml itself in
p.Systems
, and uses the same set of interfaces that were detected for the internal ceph cluster network, since they have the same restrictions. Also, rather than specifying a CIDR per system, instead we can specify the encap IP directly, and validate that it fits in one of the available subnets.Preseed tests have also been updated for the new key, which is prefixed with
ovn_
now to be clear what it affects. Thecephfs
key has also been moved into the global ceph configuration because it's not system-specific.