Skip to content

Include new checks for Grafana, Argo and Dask #563

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Include new checks for Grafana, Argo and Dask #563

wants to merge 2 commits into from

Conversation

viniciusdc
Copy link
Contributor

Reference Issues or PRs

What does this implement/fix?

Put a x in the boxes that apply

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds a feature)
  • Breaking change (fix or feature that would cause existing features not to work as expected)
  • Documentation Update
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no API changes)
  • Build related changes
  • Other (please describe):

Testing

  • Did you test the pull request locally?
  • Did you add new tests?

Documentation

Access-centered content checklist

Text styling

  • The content is written with plain language (where relevant).
  • If there are headers, they use the proper header tags (with only one level-one header: H1 or # in markdown).
  • All links describe where they link to (for example, check the Nebari website).
  • This content adheres to the Nebari style guides.

Non-text content

  • All content is represented as text (for example, images need alt text, and videos need captions or descriptive transcripts).
  • If there are emojis, there are not more than three in a row.
  • Don't use flashing GIFs or videos.
  • If the content were to be read as plain text, it still makes sense, and no information is missing.

Any other comments?

@netlify Netlify
Copy link

netlify bot commented Mar 14, 2025 •
edited
Loading

✅ Deploy Preview for nebari-docs ready!

Name Link
🔨 Latest commit 13cddbb
🔍 Latest deploy log https://app.netlify.com/sites/nebari-docs/deploys/67dc3e5c057378000874de55
😎 Deploy Preview https://deploy-preview-563--nebari-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@viniciusdc
Copy link
Contributor Author

After nebari-dev/nebari#2978 is merged this one can be as well

marcelovilla
marcelovilla requested changes Mar 17, 2025
View reviewed changes
docs/docs/how-tos/setup-healthcheck.md
@@ -61,3 +61,6 @@ Below is an explanation of the available healthchecks. This list may not be comp
| daemonset | verifies that a daemonset can be created, fully provisioned, and torn down. This checks the full kubelet functionality of every node in your Kubernetes cluster |
| deployment | verifies that a fresh deployment can run, deploy multiple pods, pass traffic, do a rolling update (without dropping connections), and clean up successfully |
| keycloak-http-check | verifies Keycloak is accessible |
| argo-http-check | verifies Argo Workflows Server is accessible |
| dask-gateway-http-check | verifies Dask Gateway Gateway API is running |
Copy link
Member

Choose a reason for hiding this comment

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

There's an extra "Gateway" in Dask Gateway

Copy link
Contributor Author

Choose a reason for hiding this comment

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

well, that's the name of the check since its the name of the actual service listed in keycloak.I think its better to list the exact match since that's whats deployed, but if we just want to raise awareness I am fine with keeping the usual names

Copy link
Member

Choose a reason for hiding this comment

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

Right, that's the name of the k8s service but I feel that's internal and users reading the documentation don't need to know the exact name but rather what "Nebari service" it refers to. That said, if you want to leave it like this, I don't have a problem with that.

docs/docs/how-tos/setup-healthcheck.md Outdated
@@ -61,3 +61,6 @@ Below is an explanation of the available healthchecks. This list may not be comp
| daemonset | verifies that a daemonset can be created, fully provisioned, and torn down. This checks the full kubelet functionality of every node in your Kubernetes cluster |
| deployment | verifies that a fresh deployment can run, deploy multiple pods, pass traffic, do a rolling update (without dropping connections), and clean up successfully |
| keycloak-http-check | verifies Keycloak is accessible |
| argo-http-check | verifies Argo Workflows Server is accessible |
| dask-gateway-http-check | verifies Dask Gateway Gateway API is running |
| grafa-http-check | verifies Grafana is accessible |
Copy link
Member

Choose a reason for hiding this comment

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

The name is grafana-http-check

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ups, that's a typo thanks

Copy link
Member

Choose a reason for hiding this comment

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

Can you push a fix with the correct name?

docs/docs/how-tos/setup-healthcheck.md
@@ -61,3 +61,6 @@ Below is an explanation of the available healthchecks. This list may not be comp
| daemonset | verifies that a daemonset can be created, fully provisioned, and torn down. This checks the full kubelet functionality of every node in your Kubernetes cluster |
| deployment | verifies that a fresh deployment can run, deploy multiple pods, pass traffic, do a rolling update (without dropping connections), and clean up successfully |
| keycloak-http-check | verifies Keycloak is accessible |
| argo-http-check | verifies Argo Workflows Server is accessible |
Copy link
Member

Choose a reason for hiding this comment

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

I'd just keep Argo Workflows instead of Argo Workflows Server here but I'll leave that up to you

@viniciusdc viniciusdc requested a review from marcelovilla March 19, 2025 14:15
@viniciusdc
Copy link
Contributor Author

Hey @marcelovilla I left a few comments to your suggestions, can you have another look?

@marcelovilla marcelovilla moved this from New 🚦 to TODO 📬 in 🪴 Nebari Project Management Mar 19, 2025
@marcelovilla marcelovilla self-assigned this Mar 19, 2025
@marcelovilla marcelovilla moved this from TODO 📬 to Changes requested 🧱 in 🪴 Nebari Project Management Mar 19, 2025
viniciusdc
viniciusdc commented Mar 20, 2025
View reviewed changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marcelovilla marcelovilla Awaiting requested review from marcelovilla

Requested changes must be addressed to merge this pull request.

Assignees

@marcelovilla marcelovilla

Labels
None yet
Projects
📖 - Documentation
Status: Todo 📬
🪴 Nebari Project Management
Status: Changes requested 🧱
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@viniciusdc @marcelovilla