csi-addons / volume-replication-operator

Apache License 2.0
16 stars 23 forks source link

Add conditions in status of volume replication #48

Closed iamniting closed 3 years ago

iamniting commented 3 years ago

As of now, we have only the State field in the status of volume replication. Which does not give us much clarity on what is the current state. Adding conditions in status will give a clear picture of the volume replication state.

iamniting commented 3 years ago

We can do something like this. We will mark the Spec.ReplicaState condition True when we met the state and others as false.

status:
  conditions:
    - lastTransitionTime: "2019-10-22T16:29:24Z"
      status: "True"
      type: IsPrimary
    - lastTransitionTime: "2019-10-22T16:29:24Z"
      status: "False"
      type: IsSecondary
    - lastTransitionTime: "2019-10-22T16:29:31Z"
      status: "False"
      type: IsResyncing
iamniting commented 3 years ago

@ShyamsundarR @leseb any thoughts on this.

ShyamsundarR commented 3 years ago

Conditions are "Condition contains details for one aspect of the current state of this API Resource.". Hence, IMO we should not make the conditions the state itself.

Why not go with a status.replicaState and the following status.conditions

status.replicaState: [Primary|Secondary|Unknown] NOTE: There would be other status variables here (like last sync time maybe), but just putting forth one for related conditions status.conditions:

iamniting commented 3 years ago

@ShyamsundarR Can you pls elaborate on the Degraded

ShyamsundarR commented 3 years ago

Here is where I think "Degraded" maybe useful.

Periodically we may check the image status to update last snapshot sync time etc. During this if either the snapshots are not syncing or other errors are noted, a degraded condition may help.

I also wanted to add a note on what the user expectation could be, here it is,

sp98 commented 3 years ago

fixed in #61