Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

feat(site): warn on provisioner health during builds#15589

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
SasSwart merged 18 commits intomainfromjjs/15048-fe
Nov 28, 2024
Merged

Conversation

SasSwart
Copy link
Contributor

@SasSwartSasSwart commentedNov 19, 2024•
edited
Loading

This PR adds warning alerts to log drawers for templates and template versions. warning alerts for workspace builds to follow in a subsequent PR. Phrasing to be finalised. Stories added and manually verified. See screenshots below.

Updating a template version with no provisioners:
Screenshot 2024-11-27 at 11 06 28
Build Errors for template versions now show tags as well:
Screenshot 2024-11-27 at 11 07 01
Updating a template version with provisioners that are busy or unresponsive:
Screenshot 2024-11-27 at 11 06 40
Creating a new template with provisioners that are busy or unresponsive:
Screenshot 2024-11-27 at 11 08 55
Creating a new template when there are no provisioners to do the build:
Screenshot 2024-11-27 at 11 08 45

No logic or api interaction yet. Just checking presentation and enumerating error cases.
@SasSwartSasSwart changed the titleadd alert to the create workspace pagefeat(site): warn on provisioner health during buildsNov 21, 2024
@bpmct
Copy link
Member

bpmct commentedNov 26, 2024•
edited
Loading

For the "no provisioners to access this template" error, can we explicitly mention which tag(s) it is expecting?

image

@SasSwart
Copy link
ContributorAuthor

For the "no provisioners to access this template" error, can we explicitly mention which tag(s) it is expecting?

image

Yes! Cian has already done that in the CLI and I plan to match it here.

bpmct reacted with thumbs up emoji

@SasSwart
Copy link
ContributorAuthor

SasSwart commentedNov 27, 2024•
edited
Loading

For the "no provisioners to access this template" error, can we explicitly mention which tag(s) it is expecting?

See updated screenshots. We now include tags and use one component for all these alerts to ensure consistency. I used an existing ProvisionerTag component to render the tags in accordance with how we render them elsewhere.

TODO before we can release this:

@SasSwartSasSwart marked this pull request as ready for reviewNovember 27, 2024 09:25
@dannykopping
Copy link
Contributor

Deployment happens automatically on merge, but we have a way to publish "preview" deployments.
Hit me up on Slack and I'll happily assist

@SasSwartSasSwart removed the request for review fromBrunoQuaresmaNovember 27, 2024 10:14
@github-actionsGitHub Actions
Copy link

github-actionsbot commentedNov 27, 2024•
edited
Loading

✔️ PR 15589 Updated successfully.
🚀 Access the credentialshere.

cc:@SasSwart

github-actions[bot] reacted with rocket emoji

Copy link
Member

@bpmctbpmct left a comment

Choose a reason for hiding this comment

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

Copy LGTM! Just left a few nits, but non-blocking. The messaging is clear, which is most important.

case matchingProvisioners === 0:
title = "Provisioning Cannot Proceed";
detail =
"There are no provisioners that accept the required tags. Please contact your administrator. Once a compatible provisioner becomes available, provisioning will continue.";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"There are no provisioners that accept the required tags. Please contact your administrator. Once a compatible provisioner becomes available,provisioning will continue.";
"There are no provisioners that accept the required tags. Please contact your administrator. Once a compatible provisioner becomes available,your build will continue.";

Also, are we able to link to thedocs on external provisioners?

If the user is an admin, it may be nice to remove "Contact your administrator." And also link to theprovisioners page for the given organization.

Copy link
Member

Choose a reason for hiding this comment

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

OK to do this as a follow-up? I've seen plenty of systems where a "contact your administrator" is shown regardless of your user's capabilities.

SasSwart reacted with thumbs up emoji
case availableProvisioners === 0:
title = "Provisioning Delayed";
detail =
"Provisioners that accept the required tags are currently anavailable. This may delay your build. Please contact your administrator if your build does not complete.";
Copy link
Member

Choose a reason for hiding this comment

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

Are we able to show a queue length, like we do in the workspaces page? If nontrivial, this copy LGTM!

Copy link
Member

Choose a reason for hiding this comment

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

I think it might be non-trivial so let's leave it as a follow-up?

SasSwart reacted with thumbs up emoji
case availableProvisioners === 0:
title = "Provisioning Delayed";
detail =
"Provisioners that accept the required tags are currently anavailable. This may delay your build. Please contact your administrator if your build does not complete.";
Copy link
Member

Choose a reason for hiding this comment

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

I think it might be non-trivial so let's leave it as a follow-up?

SasSwart reacted with thumbs up emoji
case matchingProvisioners === 0:
title = "Provisioning Cannot Proceed";
detail =
"There are no provisioners that accept the required tags. Please contact your administrator. Once a compatible provisioner becomes available, provisioning will continue.";
Copy link
Member

Choose a reason for hiding this comment

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

OK to do this as a follow-up? I've seen plenty of systems where a "contact your administrator" is shown regardless of your user's capabilities.

SasSwart reacted with thumbs up emoji
@@ -49,6 +49,73 @@ type Story = StoryObj<typeof TemplateVersionEditor>;

export const Example: Story = {};

export const UndefinedLogs: Story = {
Copy link
Member

Choose a reason for hiding this comment

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

👍

@SasSwartSasSwart merged commit56c792a intomainNov 28, 2024
28 checks passed
@SasSwartSasSwart deleted the jjs/15048-fe branchNovember 28, 2024 14:58
@johnstcn
Copy link
Member

Relates to#15048

@bpmct
Copy link
Member

@johnstcn@SasSwart I think this actually closes#15048, right? or is there more planned work?

@SasSwart
Copy link
ContributorAuthor

@johnstcn@SasSwart I think this actually closes#15048, right? or is there more planned work?

Hmm. I thought it did, but another look at#15048 prompted me to notice that we need to do this for workspace builds as well. So far we only have it for templates and template versions. The frontend for workspace builds should be quick work, given that we have the components for it already. Not sure about the backend side, but I don't anticipate any dragons.

I'll wack together a PR and get it in. People are going to start going on weekend soon. If it's as small as I think it's going to be, is there any objection to me hotfixing it into main to get it onto dogfood so we can smoke test over the weekend? I'll exercise the necessary professional judgement and defer for another engineer if it's at all complex.

@johnstcn
Copy link
Member

@SasSwart let's collaborate on this.

SasSwart reacted with thumbs up emoji

stirby pushed a commit that referenced this pull requestDec 2, 2024
This PR adds warning alerts to log drawers for templates and templateversions. warning alerts for workspace builds to follow in a subsequentPR. Phrasing to be finalised. Stories added and manually verified. Seescreenshots below.Updating a template version with no provisioners:<img width="1250" alt="Screenshot 2024-11-27 at 11 06 28"src="https://github.com/user-attachments/assets/47aa0940-57a8-44e1-b9a3-25a638fa2c8d">Build Errors for template versions now show tags as well:<img width="1250" alt="Screenshot 2024-11-27 at 11 07 01"src="https://github.com/user-attachments/assets/566e5339-0fe1-4cf7-8eab-9bf4892ed28a">Updating a template version with provisioners that are busy orunresponsive:<img width="1250" alt="Screenshot 2024-11-27 at 11 06 40"src="https://github.com/user-attachments/assets/71977c8c-e4ed-457f-8587-2154850e7567">Creating a new template with provisioners that are busy or unresponsive:<img width="819" alt="Screenshot 2024-11-27 at 11 08 55"src="https://github.com/user-attachments/assets/bda11501-b482-4046-95c5-feabcd1ad7f5">Creating a new template when there are no provisioners to do the build:<img width="819" alt="Screenshot 2024-11-27 at 11 08 45"src="https://github.com/user-attachments/assets/e4279ebb-399e-4c6e-86e2-ead8f3ac7605">(cherry picked from commit56c792a)
stirby added a commit that referenced this pull requestDec 3, 2024
-#15589-#15683-#15671---------Co-authored-by: Hugo Dutka <hugo@coder.com>Co-authored-by: Sas Swart <sas.swart.cdk@gmail.com>Co-authored-by: Spike Curtis <spike@coder.com>Co-authored-by: Cian Johnston <cian@coder.com>
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@EdwardAngertEdwardAngertEdwardAngert left review comments

@bpmctbpmctbpmct left review comments

@aslilacaslilacaslilac approved these changes

@johnstcnjohnstcnjohnstcn approved these changes

Assignees

@SasSwartSasSwart

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

6 participants
@SasSwart@bpmct@dannykopping@johnstcn@aslilac@EdwardAngert
[8]ページ先頭
©2009-2025 Movatter.jp