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

RHIDP-4566: document how to manage PVCs in RHDH operator #702

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

hmanwani-rh
Copy link
Member

IMPORTANT: Do Not Merge - To be merged by Docs Team Only

Version(s):
1.4

Issue:
RHIDP-4566

Link to docs preview:
Find the link within the PR

Reviews:

  • SME: @rm3l
  • QE: @ mention assignee
  • Docs review: @ mention assignee
  • Additional review: @mention assignee (by writer)

Additional information:

@rhdh-bot
Copy link
Collaborator

Starting from v1alpha3 (Operator version 0.4.0), you can mount directories from pre-created PVCs using the `spec.application.extraFiles.pvcs` field. PVCs are mounted as directories to the container's path, based on the following logic:

* If `spec.application.extraFiles.pvcs[].mountPath` is defined, the PVC is mounted to the specified path.
* If `spec.application.extraFiles.pvcs[].mountPath` is not defined, the PVC is mounted under `spec.application.extraFiles.mountPath/`.
Copy link
Member

Choose a reason for hiding this comment

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

if neither above defined, use RHDH container WorkingDir (if defined) <- this is a new change

application:
extraFiles:
mountPath: /my/path
pods:
Copy link
Member

Choose a reason for hiding this comment

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

pvcs, not pods <- typo in upstream doc

@@ -96,6 +98,59 @@ spec:
claimName: dynamic-plugins-root
----

Starting from v1alpha3 (Operator version 0.4.0), you can mount directories from pre-created PVCs using the `spec.application.extraFiles.pvcs` field. PVCs are mounted as directories to the container's path, based on the following logic:
Copy link
Member

@rm3l rm3l Nov 13, 2024

Choose a reason for hiding this comment

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

Suggested change
Starting from v1alpha3 (Operator version 0.4.0), you can mount directories from pre-created PVCs using the `spec.application.extraFiles.pvcs` field. PVCs are mounted as directories to the container's path, based on the following logic:
Starting from `v1alpha3`, you can mount directories from pre-created PVCs using the `spec.application.extraFiles.pvcs` field. PVCs are mounted as directories to the container's path, based on the following logic:

0.4 is the upstream operator version, but the downstream version is the RHDH version (1.4). Not sure we need to specify it here.

@@ -96,6 +98,59 @@ spec:
claimName: dynamic-plugins-root
----

Starting from v1alpha3 (Operator version 0.4.0), you can mount directories from pre-created PVCs using the `spec.application.extraFiles.pvcs` field. PVCs are mounted as directories to the container's path, based on the following logic:
Copy link
Member

@rm3l rm3l Nov 13, 2024

Choose a reason for hiding this comment

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

Just curious why this content is located here..
This is currently under Chapter 11. Configuring Red Hat Developer Hub deployment, which IIUC seems to talk more about the spec.deployment.patch Custom Resource field (introduced in the v1alpha2 CRD version, as mentioned in the chapter intro).
So I'm wondering if this should not be somewhere else (but not sure where).

Copy link
Member Author

Choose a reason for hiding this comment

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

The content is currently located here as it describes PVCs within the RHDH operator, which aligns with this section. However, would you suggest adding a subsection specifically for mounting directories from PVCs here?

Copy link
Member

@rm3l rm3l Nov 13, 2024

Choose a reason for hiding this comment

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

The content is currently located here as it describes PVCs within the RHDH operator, which aligns with this section

But this whole Chapter 11 is more about the spec.deployment.patch field with a lot of different examples for different cases, but with all of them updating the same field.

image

Copy link
Member

Choose a reason for hiding this comment

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

However, would you suggest adding a subsection specifically for mounting directories from PVCs here?

Would a different section on adding extra files make more sense here? And a subsection on mounting extra-files from PVCs? Later, we can also document the mounting of ConfigMaps or Secrets as extra-files.

Copy link
Member

@rm3l rm3l Nov 13, 2024

Choose a reason for hiding this comment

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

But if you think what we have makes sense this way, let's go with that ;-)

.Example PVCs mounted to the container
[source,yaml]
----
spec:
Copy link
Member

Choose a reason for hiding this comment

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

Would it make sense to also show the API version, Kind and metadata in this example?

Suggested change
spec:
apiVersion: rhdh.redhat.com/v1alpha3
kind: Backstage
metadata:
name: developer-hub
spec:

Copy link
Member

Choose a reason for hiding this comment

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

Well, it is two different "styles" IMO
for upstream I'd prefer fragment, this way it is clear where to put it to existed manifest w/o redundant information.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I agree with the different styles, but wanted this to be consistent with the rest of the examples listed in this section: https://redhat-developer.github.io/red-hat-developers-documentation-rhdh/pr-702/admin-rhdh/#proc-rhdh-deployment-config_assembly-admin-templates
(And having at least the API Version lets users know what is expected, I think)

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.

5 participants