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

docs: Add installation option description #11735

Merged
merged 4 commits into from
Sep 13, 2023

Conversation

qudtjs0753
Copy link
Contributor

@qudtjs0753 qudtjs0753 commented Sep 3, 2023

Fixes #7838

Motivation

  • To give clarity about namespace install and managed namespace install description in operator manual

Modifications

  • I update content in warning message(namespace install vs managed namespace install)
image

Verification

.

@qudtjs0753 qudtjs0753 closed this Sep 3, 2023
@qudtjs0753 qudtjs0753 reopened this Sep 3, 2023
Signed-off-by: KBS <qudtjs0753@naver.com>
@agilgur5 agilgur5 added the area/docs Incorrect, missing, or mistakes in docs label Sep 4, 2023
Copy link
Member

@agilgur5 agilgur5 left a comment

Choose a reason for hiding this comment

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

I've only used a cluster-scoped install, so this looks like a nice addition to me!

Made some copy-edits below

@@ -30,6 +30,12 @@ Determine your base installation option.
* A **namespace install** only executes workflows in the namespace it is installed in (typically `argo`). Look for `namespace-install.yaml` in the [release assets](https://github.com/argoproj/argo-workflows/releases/latest).
* A **managed namespace install**: only executes workflows in a specific namespace ([learn more](managed-namespace.md)).

!!! Warning "namespace install vs. managed namespace install"
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
!!! Warning "namespace install vs. managed namespace install"
!!! Info "namespace install vs. managed namespace install"

I think info makes more sense here as opposed to a warning. There is nothing to warn about, this is just descriptive

@@ -30,6 +30,12 @@ Determine your base installation option.
* A **namespace install** only executes workflows in the namespace it is installed in (typically `argo`). Look for `namespace-install.yaml` in the [release assets](https://github.com/argoproj/argo-workflows/releases/latest).
* A **managed namespace install**: only executes workflows in a specific namespace ([learn more](managed-namespace.md)).

!!! Warning "namespace install vs. managed namespace install"
A namespace install allows Workflows to run only in the namespace where Argo Workflows is installed. A managed namespace install allows Workflows to run only in one namespace besides the one where Argo Workflows is installed. Using a managed namespace install might make sense if you want some users/processes to be able to run Workflows without granting them any privileges in the namespace where Argo Workflows is installed.
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
A namespace install allows Workflows to run only in the namespace where Argo Workflows is installed. A managed namespace install allows Workflows to run only in one namespace besides the one where Argo Workflows is installed. Using a managed namespace install might make sense if you want some users/processes to be able to run Workflows without granting them any privileges in the namespace where Argo Workflows is installed.
A namespace install allows Workflows to run only in the namespace where Argo Workflows is installed.
A managed namespace install allows Workflows to run only in one namespace _other than_ the one where Argo Workflows is installed.
You can use a managed namespace install if you want some users or services to run Workflows without granting them privileges in the namespace where Argo Workflows is installed.
  • Use simpler & more direct language
  • "processes" -> "services", so as to not confuse folks with Unix processes. "Service" as in "ServiceAccount" fits better in k8s
  • tiny formatting changes

Copy link
Member

Choose a reason for hiding this comment

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

The last sentence here can be moved to managed-namespace.md. Could put it in the same paragraph as the example that is to be moved.

The first two sentences I think are redundant with managed-namespace.md

A namespace install allows Workflows to run only in the namespace where Argo Workflows is installed. A managed namespace install allows Workflows to run only in one namespace besides the one where Argo Workflows is installed. Using a managed namespace install might make sense if you want some users/processes to be able to run Workflows without granting them any privileges in the namespace where Argo Workflows is installed.

For example, if you only run CI/CD-related Workflows that are maintained by the same team that manages the Argo Workflows installation, it's probably reasonable to use a namespace install. But if all the Workflows are run by a separate data science team, it probably makes sense to give them a data-science-workflows namespace and run a "managed namespace install" of Argo Workflows from another namespace.
To configure a managed namespace install, edit the workflow-controller and argo-server Deployments to pass the --managed-namespace argument.
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
To configure a managed namespace install, edit the workflow-controller and argo-server Deployments to pass the --managed-namespace argument.
To configure a managed namespace install, edit the `workflow-controller` and `argo-server` Deployments to pass the `--managed-namespace` flag.
  • add backticks around naming
  • "argument" -> "flag"

Copy link
Member

Choose a reason for hiding this comment

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

managed-namespace.md describes this better with a YAML code block, so I don't this is needed

!!! Warning "namespace install vs. managed namespace install"
A namespace install allows Workflows to run only in the namespace where Argo Workflows is installed. A managed namespace install allows Workflows to run only in one namespace besides the one where Argo Workflows is installed. Using a managed namespace install might make sense if you want some users/processes to be able to run Workflows without granting them any privileges in the namespace where Argo Workflows is installed.

For example, if you only run CI/CD-related Workflows that are maintained by the same team that manages the Argo Workflows installation, it's probably reasonable to use a namespace install. But if all the Workflows are run by a separate data science team, it probably makes sense to give them a data-science-workflows namespace and run a "managed namespace install" of Argo Workflows from another namespace.
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
For example, if you only run CI/CD-related Workflows that are maintained by the same team that manages the Argo Workflows installation, it's probably reasonable to use a namespace install. But if all the Workflows are run by a separate data science team, it probably makes sense to give them a data-science-workflows namespace and run a "managed namespace install" of Argo Workflows from another namespace.
For example, if you only run CI/CD Workflows that are maintained by the same team that manages the Argo Workflows installation, you may want a namespace install.
But if all the Workflows are run by a separate data science team, you may want to give them a "data-science-workflows" namespace and use a managed namespace install of Argo Workflows in another namespace.
  • Use simpler & more direct language
  • "managed namespace install" is not quoted anywhere else, so remove quotes there. instead quote "data-science-workflows"
  • tiny formatting changes

Copy link
Member

Choose a reason for hiding this comment

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

Let's move this to managed-namespace.md instead

@agilgur5
Copy link
Member

agilgur5 commented Sep 4, 2023

Oh, this is pretty much a verbatim copy+paste of Crenshaw's SO Answer linked in #7838. That would have been good to make clear & credit. Some of the formatting I added above is also present in the SO Answer

Copy link
Member

@agilgur5 agilgur5 left a comment

Choose a reason for hiding this comment

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

Upon second glance, this documentation already exists in the above linked managed-namespace.md: https://argoproj.github.io/argo-workflows/managed-namespace/.
There is a more detailed configuration description there as well.

As a result, I think much of this is redundant, but we can make the link clearer and move the usage example to managed-namespace.md

!!! Warning "namespace install vs. managed namespace install"
A namespace install allows Workflows to run only in the namespace where Argo Workflows is installed. A managed namespace install allows Workflows to run only in one namespace besides the one where Argo Workflows is installed. Using a managed namespace install might make sense if you want some users/processes to be able to run Workflows without granting them any privileges in the namespace where Argo Workflows is installed.

For example, if you only run CI/CD-related Workflows that are maintained by the same team that manages the Argo Workflows installation, it's probably reasonable to use a namespace install. But if all the Workflows are run by a separate data science team, it probably makes sense to give them a data-science-workflows namespace and run a "managed namespace install" of Argo Workflows from another namespace.
Copy link
Member

Choose a reason for hiding this comment

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

Let's move this to managed-namespace.md instead

A namespace install allows Workflows to run only in the namespace where Argo Workflows is installed. A managed namespace install allows Workflows to run only in one namespace besides the one where Argo Workflows is installed. Using a managed namespace install might make sense if you want some users/processes to be able to run Workflows without granting them any privileges in the namespace where Argo Workflows is installed.

For example, if you only run CI/CD-related Workflows that are maintained by the same team that manages the Argo Workflows installation, it's probably reasonable to use a namespace install. But if all the Workflows are run by a separate data science team, it probably makes sense to give them a data-science-workflows namespace and run a "managed namespace install" of Argo Workflows from another namespace.
To configure a managed namespace install, edit the workflow-controller and argo-server Deployments to pass the --managed-namespace argument.
Copy link
Member

Choose a reason for hiding this comment

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

managed-namespace.md describes this better with a YAML code block, so I don't this is needed

@@ -30,6 +30,12 @@ Determine your base installation option.
* A **namespace install** only executes workflows in the namespace it is installed in (typically `argo`). Look for `namespace-install.yaml` in the [release assets](https://github.com/argoproj/argo-workflows/releases/latest).
* A **managed namespace install**: only executes workflows in a specific namespace ([learn more](managed-namespace.md)).

!!! Warning "namespace install vs. managed namespace install"
A namespace install allows Workflows to run only in the namespace where Argo Workflows is installed. A managed namespace install allows Workflows to run only in one namespace besides the one where Argo Workflows is installed. Using a managed namespace install might make sense if you want some users/processes to be able to run Workflows without granting them any privileges in the namespace where Argo Workflows is installed.
Copy link
Member

Choose a reason for hiding this comment

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

The last sentence here can be moved to managed-namespace.md. Could put it in the same paragraph as the example that is to be moved.

The first two sentences I think are redundant with managed-namespace.md

@@ -30,6 +30,12 @@ Determine your base installation option.
* A **namespace install** only executes workflows in the namespace it is installed in (typically `argo`). Look for `namespace-install.yaml` in the [release assets](https://github.com/argoproj/argo-workflows/releases/latest).
* A **managed namespace install**: only executes workflows in a specific namespace ([learn more](managed-namespace.md)).
Copy link
Member

@agilgur5 agilgur5 Sep 4, 2023

Choose a reason for hiding this comment

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

Suggested change
* A **managed namespace install**: only executes workflows in a specific namespace ([learn more](managed-namespace.md)).
* A **managed namespace install**: only executes workflows in a separate namespace from the one it is installed in. See [Managed Namespace](managed-namespace.md) for more details.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your detailed feedback @agilgur5! I applied your suggestion, so result are as follows.

This is managed-namespace.md
image

This one is changed description with link of managed-namespace.md in installation.md
image

Copy link
Member

Choose a reason for hiding this comment

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

Awesome, looks great and thanks for providing the screenshots too!

Move description to managed-namespace.md.

Delete obsolete description and use simpler language.

Make the link more clearer

Signed-off-by: KBS <qudtjs0753@naver.com>
Copy link
Member

@agilgur5 agilgur5 left a comment

Choose a reason for hiding this comment

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

Just one tiny improvement below, otherwise LGTM. Thanks for adding this!

docs/managed-namespace.md Outdated Show resolved Hide resolved
Co-authored-by: Anton Gilgur <4970083+agilgur5@users.noreply.github.com>
Signed-off-by: KBS <qudtjs0753@naver.com>
@agilgur5
Copy link
Member

agilgur5 commented Sep 4, 2023

CI looks like it's failing on unrelated test flakes 😕 Will need a retry from an admin

@terrytangyuan terrytangyuan enabled auto-merge (squash) September 11, 2023 00:23
@terrytangyuan terrytangyuan changed the title docs: Add installation option descrption docs: Add installation option description Sep 13, 2023
@terrytangyuan terrytangyuan enabled auto-merge (squash) September 13, 2023 22:53
@terrytangyuan terrytangyuan merged commit ab35c98 into argoproj:master Sep 13, 2023
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/docs Incorrect, missing, or mistakes in docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve managed namespace docs
3 participants