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

Enable showing the progression of a pending installation. #4247

Merged
merged 2 commits into from
Feb 8, 2022

Conversation

absoludity
Copy link
Contributor

Description of the change

Currently, when a flux2 package is installed, the installed package is created but remains in the pending state until the reconciliation completes. As it was, the ApplicationStatus component did not display the pie chart while the app was in the pending state and the app.status never updates in the UX, yet we have all the data required to display the pie chart at this point (ie. we can fetch the resources already etc.)

There is a second issue which I'll deal with in a fdlow-up PR, in that the app status never updates in the view. It seems we stream the resources and so get all the updates there, but for the straight helm case, there was never a need to re-fetch the app (the helm release). The only effect of this in the UX currently is that for this flux case, where a newly installed package is initially in the pending status, the buttons (upgrade/delete) never re-enable.

Benefits

When installing a package with flux, we now see the pie chart as soon as the resources have been fetched and can see it progress to completion.

Possible drawbacks

I need to check whether we use this pending state for the straight helm case. From memory we may see it in the UX when there are pre-install hooks, although, if that's the case, I'd expect you'd need to refresh the page (due to the issue above of the app status not updating in the UX). Even if so, I then suspect we'd simply display "No resources found" until the state changed in helm itself and the resources were created. Hopefully CI will show whether it still works.

Applicable issues

Additional information

@@ -134,10 +134,15 @@ export default function ApplicationStatus({
</div>
);
}
if (info && info.status) {
if (info?.status?.reason) {
// If the status code is different than "Deployed", display that status
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't we update this comment to include the pending status added, or it is not related to the if condition modified?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Woops, yep, I'll update it. Thanks.

@castelblanque
Copy link
Collaborator

Great to have better UI feedback, thanks!

@absoludity absoludity force-pushed the 3695-check-installed-pkg-details-6 branch 2 times, most recently from 5b35ec8 to 8f07b43 Compare February 8, 2022 11:22
Base automatically changed from 3695-check-installed-pkg-details-6 to main February 8, 2022 18:49
Signed-off-by: Michael Nelson <minelson@vmware.com>
Signed-off-by: Michael Nelson <minelson@vmware.com>
@absoludity absoludity force-pushed the 3695-check-installed-pkg-details-7 branch from b4db2be to 9835815 Compare February 8, 2022 18:53
@absoludity absoludity merged commit 16a029a into main Feb 8, 2022
@absoludity absoludity deleted the 3695-check-installed-pkg-details-7 branch February 8, 2022 19:44
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.

None yet

2 participants