Set NodeSet.Status.DeployedVersion when update service is complete #899
Conversation
pkg/openstack/dataplane.go
Outdated
|
|
||
| // DataplaneNodesetsUpdated returns true if all nodesets are deployed with the latest version | ||
| func DataplaneNodesetsUpdated(ctx context.Context, helper *helper.Helper, version *corev1beta1.OpenStackVersion, dataplaneNodesets *dataplanev1.OpenStackDataPlaneNodeSetList) bool { |
There was a problem hiding this comment.
this will get called repeatedly during a minor update until it reconciles. while this may work in some cases it also depends on DataplaneNodeSet to trigger a reconcile loop on OpenStackVersion (this is the only resource we are watching) and it is not as efficient as perhaps just having the OpenStackDataplaneNodeset controller do it (since it already watches Deployment anyway).
There was a problem hiding this comment.
Ok, I have reworked it to set a condition on the NodeSet instead, and just updated the func here to check that condition.
slagle
force-pushed
the
dataplane-condition-update
branch
2 times, most recently
from
c108b57 to
c19f8a1
Compare
slagle
changed the title
pkg/openstack/dataplane.go
Outdated
| @@ -62,7 +62,7 @@ func DataplaneNodesetsDeployed(version *corev1beta1.OpenStackVersion, dataplaneN | |||
| if !nodeset.IsReady() { | |||
| return false | |||
| } | |||
| if nodeset.Status.DeployedVersion != version.Spec.TargetVersion { | |||
| if !nodeset.Status.Conditions.IsTrue(dataplanev1.NodeSetMinorUpdateReadyCondition) { | |||
There was a problem hiding this comment.
I think we might need to check both of these right?
There was a problem hiding this comment.
The version check has already been done in the nodeset controller before the condition is set.
There was a problem hiding this comment.
while that may be true, this function gets executed from the openstackversion side of things which could bump targetVersion. I am trying to guard against the case where the version changes, but in the interim this condition hasn't yet been reset
There was a problem hiding this comment.
Reworked so that this "if" condition will work without any changes.
slagle
force-pushed
the
dataplane-condition-update
branch
from
c19f8a1 to
cfb4154
Compare
|
Build failed (check pipeline). Post https://review.rdoproject.org/zuul/buildset/032fdd4ff74b42548c172f28d801871d ❌ openstack-k8s-operators-content-provider FAILURE in 7m 51s |
slagle
force-pushed
the
dataplane-condition-update
branch
3 times, most recently
from
faacaa8 to
4837c75
Compare
slagle
changed the title
Changes setting NodeSet.Status.DeployedVersion to be only when a Deployment has completed that contains a service where EDPMServiceType == 'update'. When satisfied, the Deployment's Status.DeployedVersion will be copied to NodeSet.Status.DeployedVersion. Jira: https://issues.redhat.com/browse/OSPRH-8176 Signed-off-by: James Slagle <[email protected]>
slagle
force-pushed
the
dataplane-condition-update
branch
from
4837c75 to
c884797
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dprince, slagle The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-merge-bot
bot
merged commit 06baa21
into
openstack-k8s-operators:main
Set NodeSet.Status.DeployedVersion when update service is complete
Changes setting NodeSet.Status.DeployedVersion to be only when a
Deployment has completed that contains a service where EDPMServiceType
== 'update'. When satisfied, the Deployment's Status.DeployedVersion
will be copied to NodeSet.Status.DeployedVersion.
Jira: https://issues.redhat.com/browse/OSPRH-8176
Signed-off-by: James Slagle [email protected]