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

feat: adds status information to the LVMCluster CR #76

Merged
merged 2 commits into from
Jan 18, 2022
Merged

feat: adds status information to the LVMCluster CR #76

merged 2 commits into from
Jan 18, 2022

Conversation

nbalacha
Copy link
Contributor

The LVMCluster operator will now consolidate the information in the LVMVolumeGroupNodeStatus objects and update the LVMCluster.Status.

Copy link
Contributor

@sp98 sp98 left a comment

Choose a reason for hiding this comment

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

nit

DeviceClassStatuses []DeviceClassStatus `json:"deviceClassStatuses,omitempty"`
}

// VGNodeStatus defines the observed status of the VG across all nodes
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// VGNodeStatus defines the observed status of the VG across all nodes
// DeviceClassStatus defines the observed status of the VG across all nodes


// VGNodeStatus defines the observed status of the VG across all nodes
type DeviceClassStatus struct {
// Name is the name of the VG
Copy link
Contributor

Choose a reason for hiding this comment

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

This might cause confusion between VG and DeviceClass. I think we should use either VG or DeviceClass

type DeviceClassStatus struct {
// Name is the name of the VG
Name string `json:"name,omitempty"`
// Status tells if the VG was created on the node
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Status tells if the VG was created on the node
// NodeStatus tells if the VG was created on the node

@@ -197,6 +197,53 @@ func (r *LVMClusterReconciler) reconcile(ctx context.Context, instance *lvmv1alp
return ctrl.Result{}, nil
}

func (r *LVMClusterReconciler) updateLVMClusterStatus(ctx context.Context, instance *lvmv1alpha1.LVMCluster) error {

vgnodemap := make(map[string][]lvmv1alpha1.NodeStatus)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
vgnodemap := make(map[string][]lvmv1alpha1.NodeStatus)
vgNodeMap := make(map[string][]lvmv1alpha1.NodeStatus)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

return err
}

for _, nodeitem := range vgNodeStatusList.Items {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
for _, nodeitem := range vgNodeStatusList.Items {
for _, nodeItem := range vgNodeStatusList.Items {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

The LVMCluster.Status will now contain a list of VGs,
the nodes on which they have been created and the status on
that node.

Signed-off-by: N Balachandran <[email protected]>
The LVMCluster controller will update the LVMCluster.Status with
the consolidated status information in the LVMVolumeGroupNodeStatus
objects.

Signed-off-by: N Balachandran <[email protected]>
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 18, 2022
Copy link
Contributor

@leelavg leelavg left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 18, 2022

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: leelavg, nbalacha, sp98

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@nbalacha nbalacha merged commit 9a667b6 into openshift:main Jan 18, 2022
@nbalacha nbalacha deleted the status branch January 18, 2022 10:50
@nbalacha nbalacha restored the status branch January 18, 2022 10:50
@nbalacha nbalacha deleted the status branch March 22, 2022 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants