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

Add a guide about upgrading applications without downtime #944

Merged
merged 8 commits into from
Aug 17, 2023

Conversation

pleshakov
Copy link
Contributor

Proposed changes

Problem:
As a user of NKG, I want a guide to explain the different strategies I can upgrade my application (NOT NKG) while using NKG specific features (such as traffic splitting) So that I minimize the downtime my application encounters.

Solution:

  • Add a guide that covers NKG specific features that help upgrade applications without downtime
  • Introduce a new /docs/guide folder for guides.
  • Add a link to the guides folder from the main README.

Testing: none

Closes #704

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

@pleshakov pleshakov requested a review from a team as a code owner August 8, 2023 22:01
@github-actions github-actions bot added the documentation Improvements or additions to documentation label Aug 8, 2023
Copy link
Contributor

@sjberman sjberman left a comment

Choose a reason for hiding this comment

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

I'd also tag someone from the docs team and PM to do a review of this.

docs/guides/upgrade-apps-without-downtime.md Outdated Show resolved Hide resolved
docs/guides/upgrade-apps-without-downtime.md Outdated Show resolved Hide resolved
docs/guides/upgrade-apps-without-downtime.md Outdated Show resolved Hide resolved
docs/guides/upgrade-apps-without-downtime.md Outdated Show resolved Hide resolved
docs/guides/upgrade-apps-without-downtime.md Outdated Show resolved Hide resolved
docs/guides/upgrade-apps-without-downtime.md Outdated Show resolved Hide resolved
docs/guides/upgrade-apps-without-downtime.md Outdated Show resolved Hide resolved
docs/guides/upgrade-apps-without-downtime.md Outdated Show resolved Hide resolved
docs/guides/upgrade-apps-without-downtime.md Outdated Show resolved Hide resolved
docs/guides/upgrade-apps-without-downtime.md Outdated Show resolved Hide resolved
@pleshakov pleshakov requested a review from mpstefan August 9, 2023 14:49
Copy link
Contributor

@kate-osborn kate-osborn left a comment

Choose a reason for hiding this comment

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

Great doc! Just a couple small comments

docs/guides/upgrade-apps-without-downtime.md Outdated Show resolved Hide resolved
docs/guides/upgrade-apps-without-downtime.md Outdated Show resolved Hide resolved
docs/guides/upgrade-apps-without-downtime.md Outdated Show resolved Hide resolved
docs/guides/upgrade-apps-without-downtime.md Outdated Show resolved Hide resolved
Copy link
Contributor

@kate-osborn kate-osborn left a comment

Choose a reason for hiding this comment

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

🚀

Problem:
As a user of NKG, I want a guide to explain the different strategies
I can upgrade my application (NOT NKG) while using NKG specific features
(such as traffic splitting) So that I minimize the downtime my
application encounters.

Solution:
- Add a guide that covers NKG specific features that help upgrade
applications without downtime
- Introduce a new /docs/guide folder for guides.
- Add a link to the guides folder from the main README.

Testing: none

Closes nginxinc#704

Co-authored-by: Saylor Berman <s.berman@f5.com>

Co-authored-by: Kate Osborn <50597707+kate-osborn@users.noreply.github.com>
@pleshakov pleshakov force-pushed the doc/zero-downtime-for-apps-guide branch from 5915410 to 499ca56 Compare August 11, 2023 19:18
ADubhlaoich and others added 2 commits August 15, 2023 10:43
This commit changes the order of content in the upgrade guide, as well
as rewriting much of the text to be more concise. Some of the
footnote-style links have also been changed to be inline ones.
Copy link
Contributor

@ADubhlaoich ADubhlaoich left a comment

Choose a reason for hiding this comment

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

I have pushed a large amount of changes to the document which restructure the text and rephrase much of the content. Most of the changes were for the sake of imperative instructions and removing various qualifiers, making the text more concise.

Some of these changes, however, do not pass due to the Markdown line length checker. What convention is that specific length based on?

There are many instances of links written in a footnote notation style which is unprecedented elsewhere in NGINX; most of them could be written with regular inline markdown links.

I can push additional changes to fix the Markdown checks but they would be prioritising passing the check as opposed to the readability. You might find it useful to check out the "Content Types and Templates" internal page for an example of the information architecture used elsewhere for future docs.

@pleshakov
Copy link
Contributor Author

@ADubhlaoich

I have pushed a large amount of changes to the document which restructure the text and rephrase much of the content. Most of the changes were for the sake of imperative instructions and removing various qualifiers, making the text more concise.

👍

Some of these changes, however, do not pass due to the Markdown line length checker. What convention is that specific length based on?

the length limit is 80. We choose it for all markdown files in our repo, so that is is easier read the source file and to review.

I made a few changes based on your changes. Mainly:

  • removed "It ensures that clients attempting to access your application will receive standard response codes (such as 502) instead of empty responses during upgrades." because this is not the goal of the guide -- the goal is to prevent downtime, which are 502 responses.
  • renamed "Endpoint switching" to "Upstream Servers Updates" - to make it use NGINX terminology.
  • a few smaller changes and fixes

Please let me know if those are ok.

Copy link
Contributor

@ADubhlaoich ADubhlaoich left a comment

Choose a reason for hiding this comment

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

Approved with one minor suggestion - "Upstream Servers Updates" => "Upstream Server Updates". Updates is already pluralised and making Server singular is better for readability, with the possibility of multiple implied in context.

If the plan at a later stage is to serve the docs through a website, the markdown line length checking may become an issue since the line length is currently prioritising GitHub readability.

IDEs and web frameworks have no issue with word wrapping paragraphs of arbitrary line length, so this may become the writing equivalent of technical debt that'll need to be addressed if the default expectation of reader consumption through GitHub changes.

pleshakov and others added 2 commits August 17, 2023 13:50
Co-authored-by: Alan Dooley <ADubhlaoich@users.noreply.github.com>
@pleshakov pleshakov merged commit b2c5146 into nginxinc:main Aug 17, 2023
22 checks passed
@pleshakov pleshakov deleted the doc/zero-downtime-for-apps-guide branch August 17, 2023 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Documentation: How to upgrade your application with limited downtime using NKG
6 participants