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

Update docs to reflect Azure 2022 process #2823

Merged
merged 21 commits into from
Aug 16, 2022

Conversation

Sieboldianus
Copy link
Contributor

I recently went through the Kubernetes on Microsoft Azure Kubernetes Service (AKS) and Kubernetes on Microsoft Azure Kubernetes Service (AKS) with Autoscaling docs.

There were some minor inconsistencies with the current Azure process that I have updated.

The autoscaling feature is now in production and can be enabled with a single flag. This has been incorporated into the main azure docs.
…eprecated and only appId should be used. Update the docs accordingly.

[1]: Azure/azure-cli#19179 (comment)
@welcome
Copy link

welcome bot commented Aug 2, 2022

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please make sure you followed the pull request template, as this will help us review your contribution more quickly.
welcome
You can meet the other Jovyans by joining our Discourse forum. There is also a intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

@Sieboldianus
Copy link
Contributor Author

Sieboldianus commented Aug 2, 2022

There's a broken link outside of the scope of this PR that is responsible for failed checks:

(administrator/upgrading/upgrade-1-to-2: line  114) broken    
https://github.com/jupyterhub/zero-to-jupyterhub-k8s/blob/2.0.0/images/hub/Dockerfile - 
404 Client Error: Not Found for url: 

@consideRatio
Copy link
Member

@Sieboldianus wieeee thank you for helping out with this! I'm very positive on combining both the separate documents into one as that added significant complexity with regards to maintaining this documentation.

I'll try find time to review this tonight, but a key review point in my mind will be: was something of relevance lost when we removed one of the versions of the azure setup that should be retained in the one we keep?

@Sieboldianus
Copy link
Contributor Author

I can check again in detail, but it mostly looked duplicate, except for the preview-feature part (which isn't necessary anymore).

I'll also want to look into the part again for activating autoscale for an existing cluster, just to make sure it works as written.

Thank you for reviewing this. Thanks for the great docs!

@Sieboldianus
Copy link
Contributor Author

Sieboldianus commented Aug 3, 2022

@consideRatio: I went through the docs again and compared the autoscaler to the standard docs and merged all.

  • In the autoscaling docs, the paragraph At this stage, you may wish to think about customising your deployment. The [Hub23 Deployment Guide](https://alan-turing-institute.github.io/hub23-deploy/) contains instructions for deploying a Kubernetes cluster to Azure with [autoscaling and multiple nodepools](https://alan-turing-institute.github.io/hub23-deploy/deploy-k8s/az-aks-create.html). These instructions can be combined so that all nodepools can autoscale. was missing, but I guess this was just a result of diversion due to the duplicate handling. I left the paragraph as is.
  • Replaced az ad sp show --id http://<SERVICE-PRINCIPAL-NAME> --query appId with az ad app list --filter "displayname eq '<SERVICE-PRINCIPAL-NAME>'"- According to azure-cli issue #19179, --id http// has ben deprecated and only appId should be used.
  • In the autoscaling docs, --enable-vmss now became --vm-set-type VirtualMachineScaleSets, see: #927
  • I incorporated the part on adjusting autoscaler rules in the azure frontend to the standard docs:
    • Removed the part on "register for Microsoft Insights", since this is not necessary anymore
    • Updated the screenshots (I think it is also ok to stick with the old ones, the changes are minor, but I still created new ones)
  • Verified all commands

I think it makes sense merging both docs, since the differences between autoscaling and not autoscaling for a cluster are minimal, but I am new to the project and not the best to decide.

After installing a helm chart, kubectl outputs the command to get the public IP of the proxy. This should be used as a default in the docs:
You can find the public (load-balancer) IP of JupyterHub by running:

> kubectl -n ... get svc proxy-public -o jsonpath='{.status.loadBalancer.ingress[].ip}'
>
> It might take a few minutes for it to appear\!
@Sieboldianus
Copy link
Contributor Author

Sieboldianus commented Aug 3, 2022

While I was at it, I also updated Installing JupyterHub: Getting the public IP of the Proxy (7ff31fd).

kubectl outputs the recommended command, after installation of the helm chart:

> Release "<helm-release-name>" does not exist. Installing it now.
> NAME: <helm-release-name>
> LAST DEPLOYED: Wed Aug  3 10:01:48 2022
> NAMESPACE: <k8s-namespace>
> STATUS: deployed
> REVISION: 1
> TEST SUITE: None
> NOTES:
> Thank you for installing JupyterHub!
> 
> Your release is named "<helm-release-name>" and installed into the namespace "<k8s-namespace>".
> 
> You can check whether the hub and proxy are ready by running:
> 
>  kubectl --namespace=<k8s-namespace> get pod
> 
> and watching for both those pods to be in status 'Running'.
> 
> You can find the public (load-balancer) IP of JupyterHub by running:
> 
>   kubectl -n <k8s-namespace> get svc proxy-public -o jsonpath='{.status.loadBalancer.ingress[].ip}'
> 
> It might take a few minutes for it to appear!
> 
> To get full information about the JupyterHub proxy service run:
> 
>   kubectl --namespace=<k8s-namespace> get svc proxy-public
> 
> If you have questions, please:
> 
>   1. Read the guide at https://z2jh.jupyter.org
>   2. Ask for help or chat to us on https://discourse.jupyter.org/
>   3. If you find a bug please report it at https://github.com/jupyterhub/zero-to-jupyterhub-k8s/issues

This does not really fit in here. Happy to submit a different PR for this commit.

Copy link
Member

@consideRatio consideRatio left a comment

Choose a reason for hiding this comment

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

Thank you @Sieboldianus !!!! ❤️ 🎉

@consideRatio consideRatio merged commit 6be5267 into jupyterhub:main Aug 16, 2022
@welcome
Copy link

welcome bot commented Aug 16, 2022

Congrats on your first merged pull request in this project! 🎉
congrats
Thank you for contributing, we are very proud of you! ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants