Skip to content

Conversation

@bobfuru
Copy link
Contributor

@bobfuru bobfuru commented Apr 8, 2020

BZ1739420
Doc bug asks to include optional steps for how to reserve a PV for specific tasks, as described in BZ1717746.
Applies to enterprise-3.11 only.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this the right placement for this optional step (between Define and Deploy)? Also, not certain that "when using a selector" is necessary here?

Copy link
Contributor

Choose a reason for hiding this comment

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

The placement is OK, however, the heading does not match the content. Heading says that it's about label selectors, but the two solutions don't use selector at all.

There are two mixed topics:

  1. bind a PVC to a specific PV
  • This can be done either by specifying pvc.spec.volumeName (when knowing the PV name) or using pvc.spec.selector (when knowing PV labels).
  1. reserve a PV to specific PVC (or namespace)
  • This can be done by using a specific storage class or setting pv.spec.claimRef, as outlined correctly below.

I would not mix those in a single chapter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, @jsafrane. I understand more now that you spelled out #1.

I've added an additional section for using a selector or volumeName, so there are now two distinct chapters. Let me know if this looks good.

@bobfuru
Copy link
Contributor Author

bobfuru commented Apr 8, 2020

@jsafrane PTAL - this is based on a bug fix you documented in BZ1717746.

@bobfuru
Copy link
Contributor Author

bobfuru commented Apr 8, 2020

@liangxia or @qinpingli PTAL

@bobfuru
Copy link
Contributor Author

bobfuru commented Apr 8, 2020

@jkaurredhat PTAL

@qinpingli
Copy link

@chao007 Could you help review this patch?

Copy link
Contributor

Choose a reason for hiding this comment

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

The placement is OK, however, the heading does not match the content. Heading says that it's about label selectors, but the two solutions don't use selector at all.

There are two mixed topics:

  1. bind a PVC to a specific PV
  • This can be done either by specifying pvc.spec.volumeName (when knowing the PV name) or using pvc.spec.selector (when knowing PV labels).
  1. reserve a PV to specific PVC (or namespace)
  • This can be done by using a specific storage class or setting pv.spec.claimRef, as outlined correctly below.

I would not mix those in a single chapter.

If necessary in a multi-tenant environment, use a quota definition to reserve the storage class and PV(s) only to a specific namespace.
====

. Pre-bind the PV to your PVC using the PVC namespace and name. A PV defined as such will bind only to the specified PVC and to nothing else, as shown in the following example:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@chao007 - I dropped "namespace" from the subhead as you suggested, but is it okay to still have it "PVC namespace" here or should it also be removed?

@jsafrane
Copy link
Contributor

lgtm

@bobfuru
Copy link
Contributor Author

bobfuru commented Apr 15, 2020

@chao007 Can you please approve from QE side?

@bobfuru
Copy link
Contributor Author

bobfuru commented Apr 15, 2020

@openshift/team-documentation PTAL

@bobfuru bobfuru added the peer-review-needed Signifies that the peer review team needs to review this PR label Apr 15, 2020
@bmcelvee
Copy link
Contributor

LGTM

@bmcelvee bmcelvee added peer-review-done Signifies that the peer review team has reviewed this PR and removed peer-review-needed Signifies that the peer review team needs to review this PR labels Apr 15, 2020
@chao007
Copy link

chao007 commented Apr 16, 2020

LGTM.

@bobfuru bobfuru merged commit d5d4ff7 into openshift:enterprise-3.11 Apr 16, 2020
@bobfuru bobfuru deleted the BZ1739420 branch April 16, 2020 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

branch/enterprise-3.11 peer-review-done Signifies that the peer review team has reviewed this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants