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

Updates production deployment documentation. #473

Merged
merged 1 commit into from
Feb 5, 2020

Conversation

hardbyte
Copy link
Collaborator

@hardbyte hardbyte commented Feb 2, 2020

This started as just updating a dead hyperlink but has ended up being misc updates to the Kubernetes deployment docs.

Closes #471

@hardbyte hardbyte requested a review from wilko77 February 2, 2020 23:31
Copy link
Collaborator

@wilko77 wilko77 left a comment

Choose a reason for hiding this comment

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

apart from the comments. There are some tests failing...

docs/production-deployment.rst Show resolved Hide resolved

Minimal Deployment
------------------

To run with minikube for local testing we have provided a ``minimal.yaml`` file that will
set very small resource limits. Install the minimal system with::
To run with minikube for local testing we have provided a ``minimal.yaml`` configuration file that will
Copy link
Collaborator

Choose a reason for hiding this comment

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

where is that minimal.yaml file? I couldn't find it in the deployment folder.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmm, I would have sworn there was one. I'll remove the reference as it was so minimal it couldn't do much except deploy the service

docs/production-deployment.rst Show resolved Hide resolved
imagePullPolicy: Always
env:
- name: ENTITY_SERVICE_URL
value: https://beta.anonlink.data61.xyz/api/v1
value: https://anonlink.easd.data61.xyz/api/v1
Copy link
Collaborator

Choose a reason for hiding this comment

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

why do we define the server differently to the benchmark yaml? Is there a reason for this inconsistency?
Wouldn't it be nicer if they both accepted the same name/value pair?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure would be -> #475

@@ -15,11 +15,11 @@ spec:
restartPolicy: Never
containers:
- name: entitytester
image: quay.io/n1analytics/entity-app
image: data61/anonlink-app:v1.12.0
Copy link
Collaborator

Choose a reason for hiding this comment

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

wouldn't it be better to link to latest or stable or something similar. We don't want to update the deployment files with every release.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure, happy to use latest. It is mostly there as an example, and pinning exact versions of what runs is considered best practice.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll remove the log level setting from here too - it is just for the logging level during tests which pytest overrides anyway.

@hardbyte
Copy link
Collaborator Author

hardbyte commented Feb 3, 2020

Thanks for the review @wilko77 - have addressed the inconsistencies in the documentation and opened an issue for the env var.

@hardbyte hardbyte requested a review from wilko77 February 3, 2020 08:18
@hardbyte hardbyte merged commit a9aeb1b into develop Feb 5, 2020
@hardbyte hardbyte deleted the fix-deploment-links branch February 5, 2020 00:39
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.

Update production deployment docs
2 participants