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

Fix racing cluster FIPS settings #235

Merged
merged 1 commit into from
Apr 6, 2022
Merged

Conversation

liranr23
Copy link
Member

@liranr23 liranr23 commented Apr 5, 2022

When we have a new cluster, the FIPS mode is defined by the first host
being added to that cluster. By default the value of it when unavailable
from the host is false. In case of new host deployment to that cluster
and reboot, we may not get the capabilities as expected and even when
the host is with FIPS set, it will still wrongly set the cluster with
false, making the host non-operational until the cluster is edited by
the user to be FIPS true.

The placement the engine handles the FIPS is moved to a place we can be
certain we already get the capabilities from the host. Therefore, we
will always set it with the right value of the first host in that new
cluster.

Change-Id: I3c21028e3bd0f882340afc13ab05911fb92ec90c
Bug-Url: https://bugzilla.redhat.com/2065543
Signed-off-by: Liran Rotenberg lrotenbe@redhat.com

Copy link
Member

@mwperina mwperina left a comment

Choose a reason for hiding this comment

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

+1

@liranr23
Copy link
Member Author

liranr23 commented Apr 6, 2022

/ost

Copy link
Member

@ahadas ahadas left a comment

Choose a reason for hiding this comment

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

this change moves the handling of the FIPS mode to a better place - that's good
but the issue it fixes reveals that we don't set the initial value correctly, it's less visible because it is mostly used internally but I see that we also display it in the host general tab - now that we enforce the same FIPS mode across the cluster, it doesn't make much sense. how about removing it from there?

@liranr23
Copy link
Member Author

liranr23 commented Apr 6, 2022

this change moves the handling of the FIPS mode to a better place - that's good but the issue it fixes reveals that we don't set the initial value correctly, it's less visible because it is mostly used internally but I see that we also display it in the host general tab - now that we enforce the same FIPS mode across the cluster, it doesn't make much sense. how about removing it from there?

Yes, now it's on cluster level and used internally. There is no point of having it exposed.

@ljelinkova
Copy link
Contributor

The FE looks good, just update the commit message with the explanation why the field has been removed from host view.

When we have a new cluster, the FIPS mode is defined by the first host
being added to that cluster. By default the value of it when unavailable
from the host is `false`. In case of new host deployment to that cluster
and reboot, we may not get the capabilities as expected and even when
the host is with FIPS set, it will still wrongly set the cluster with
`false`, making the host non-operational until the cluster is edited by
the user to be FIPS `true`.

The placement the engine handles the FIPS is moved to a place we can be
certain we already get the capabilities from the host. Therefore, we
will always set it with the right value of the first host in that new
cluster.

Since the value from the host is now used on cluster level and used
internally only, this information was removed from the host general tab
in the UI.

Change-Id: I3c21028e3bd0f882340afc13ab05911fb92ec90c
Bug-Url: https://bugzilla.redhat.com/2065543
Signed-off-by: Liran Rotenberg <lrotenbe@redhat.com>
@ahadas
Copy link
Member

ahadas commented Apr 6, 2022

/ost

@michalskrivanek
Copy link
Member

lgtm. wait with ost test for build to finish

@liranr23
Copy link
Member Author

liranr23 commented Apr 6, 2022

/ost

@liranr23 liranr23 requested a review from ahadas April 6, 2022 14:38
@ahadas ahadas merged commit ec8dc7d into oVirt:master Apr 6, 2022
@ahadas ahadas added the OST label Apr 6, 2022
@liranr23 liranr23 deleted the cluster_fips_init branch April 6, 2022 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants