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

vmsingle: add status field #673

Merged
merged 4 commits into from
Jun 26, 2023
Merged

vmsingle: add status field #673

merged 4 commits into from
Jun 26, 2023

Conversation

Haleygo
Copy link
Contributor

@Haleygo Haleygo commented Jun 20, 2023

#670
Status flow:
changes detected -> status expanding
CreateOrUpdateVMSingle returned an error -> status failed with reason.
CreateOrUpdateVMSingle and other functions returned no error -> status operational and update lastAppliedSpec.

@Haleygo Haleygo requested a review from f41gh7 June 20, 2023 15:57
controllers/vmsingle_controller.go Outdated Show resolved Hide resolved
controllers/vmsingle_controller.go Show resolved Hide resolved
return result, fmt.Errorf("failed to get deployment for vmsingle %s: %w", req.NamespacedName, err)
}

instance.Status.ReadyReplicas = currentDeploy.Status.ReadyReplicas
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think, we could skip Replicas fields changing, VMSingle always has only 1 replica.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I stored ready pod replicas of deployment here to see if single is operational.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Notice this note "if you need more - use vm cluster" until now :)
Fixed, not adding ReadyReplicas fields and keep rest replicas untouched for backward compatibility.

controllers/vmsingle_controller.go Outdated Show resolved Hide resolved
@Haleygo Haleygo force-pushed the polish-vmsingle-status branch 2 times, most recently from f0318f8 to 357764d Compare June 24, 2023 03:51
Copy link
Collaborator

@f41gh7 f41gh7 left a comment

Choose a reason for hiding this comment

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

LGTM

@f41gh7 f41gh7 merged commit cf83fab into master Jun 26, 2023
3 checks passed
@f41gh7
Copy link
Collaborator

f41gh7 commented Jun 26, 2023

Thanks for contribution!

@Haleygo Haleygo deleted the polish-vmsingle-status branch June 26, 2023 11:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants