Finalize pvc support #5651

skonto · 2023-08-02T07:29:54Z

Proposed Changes

Continues the work in [WIP] Add PVC docs #5507

netlify · 2023-08-02T07:30:03Z

✅ Deploy Preview for knative ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	`138e5bc`
🔍 Latest deploy log	https://app.netlify.com/sites/knative/deploys/6548f2bdc99fa70007966c17
😎 Deploy Preview	https://deploy-preview-5651--knative.netlify.app/docs/serving/services/storage
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

ReToCode · 2023-08-03T04:45:01Z

config/nav.yml

@@ -100,6 +100,20 @@ nav:
        - Architecture: serving/architecture.md
        - Request Flow: serving/request-flow.md
      - Resources:
+        - Services:


What is the idea about moving this? I (personally) do not get what why "Resources" and "Developer Topics" are two chapters in the first place. Maybe merge them all together?

This was an idea coming from the other PR. Having just revisions didnt make much sense either, Services fall under the same category. I will take a look and see if we can have a better grouping but this could be something to handle in a different PR as well.

ReToCode · 2023-08-03T04:47:03Z

docs/serving/services/storage.md

@@ -1,12 +1,35 @@
 # Volume Support for Knative services

-By default Serving supports the mounting the [volume types](https://kubernetes.io/docs/concepts/storage/volumes): `emptyDir`, `secret`, `configMap` and `projected`. [PersistentVolumes](https://kubernetes.io/docs/concepts/storage/persistent-volumes/) are supported but require a [feature flag](../configuration/feature-flags.md) to be enabled.
+You can provide data storage for Knative Services by configuring different types of volumes.


four volume types or do you see projected as another definition of the three?

Actually I see them as four given that at the K8s api level we have:

Volume -> (name, VolumeSource) VolumeSource -> {EmptyDir, Secret, ConfigMap, Projected, DownwardAPI,...} Projected -> {DownwardAPIProjection, ConfigMapProjection, SecretProjection, ServiceAccountProjection }

Note that volumeSource DownwardAPI is not supported directly it seems.
The last PR on this area was btw the addition of the projected downwardapi.

Ah, but then the listing is a bit misleading.

different types of volumes

and in the next sentence:

volume types: emptyDir, secret, configMap and projected

Can we change one of those "types" then?

These are different type of volumes (four) according to the api. I can change to different volumes types sounds good?

ReToCode · 2023-08-03T04:48:59Z

docs/serving/services/storage.md

+
+## Prerequisites
+
+Before you can configure PVCs for a Service, this feature must be enabled in the `KnativeServing` ConfigMap:


Do we also want to document without the operator? For the yaml install a modification of the config-map?

Actually we can have a note about the operator but this is the first feature described separately, most are handled in the feature-flags page and there is no mentioning of the operator there. It is only about the config map.

in the KnativeServing ConfigMap:

But then this is not correct, right? It is either the Custom Resource KnativeServing or the config-features ConfigMap.

In general listing one option does not exclude the other. For completeness sake we should add both options.
So when you deploy Serving with the Operator your API is the Serving CR so config should be changed there.
Operator will propagate the change to the configmap as usual.
When you dont deploy Serving with the Operator configmap is your place where to change stuff.
I agree with describing the options available.

skonto · 2023-10-03T07:05:58Z

@ReToCode hi I updated the PR, the re-organization should de done in a follow up PR.

ReToCode · 2023-10-03T11:36:16Z

docs/serving/services/storage.md

+
+## Prerequisites
+
+Before you can configure PVCs for a Service, this feature must be enabled in the `KnativeServing` ConfigMap:


Sorry, this is still a bit confusing

this feature must be enabled in the KnativeServing ConfigMap

It's either KnativeServing CR OR the config-map

Also then the hint

Note that if you have installed Serving via the Knative operator then you need to set the above feature flags at the corresponding Serving CR.

Is unnecessary when the example is with the KnativeServing CR already.

Seems like in other pages we assume the YAML installation was used, so maybe it it easier to have a command here to patch the config-map, as we have here

kubectl patch configmap/config-network
--namespace knative-serving
--type merge
--patch '{"data":{"ingress-class":"kourier.ingress.networking.knative.dev"}}'

Then we can also keep the note. Ideally we'll also have a section in the operator, describing the config-overwrite mechanism in general.

Is unnecessary when the example is with the KnativeServing CR already.

@ReToCode the yaml file you see is not with the Serving CR it is meant to show how to modify the configmap I will use the patch just to clarify. An the reason I added this was also to note that people can either modify the configmap or the CR, anyway I will rephrase.

skonto · 2023-11-06T13:18:29Z

@ReToCode pls take a look.

ReToCode

/lgtm
/approve

knative-prow · 2023-11-07T07:44:25Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ReToCode, skonto

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~docs/serving/OWNERS~~ [ReToCode,skonto]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

knative-prow bot requested review from csantanapr and nak3 August 2, 2023 07:30

skonto requested review from dprotaso and removed request for csantanapr and nak3 August 2, 2023 07:30

knative-prow bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 2, 2023

ReToCode reviewed Aug 3, 2023

View reviewed changes

ReToCode reviewed Oct 3, 2023

View reviewed changes

knative-prow-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 5, 2023

skonto added 3 commits November 6, 2023 14:30

pvc support

745ff2f

comments

5b0b5d0

fixes

3ecc8bf

skonto force-pushed the pvc_fix branch from b9916e9 to 3ecc8bf Compare November 6, 2023 13:17

knative-prow-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 6, 2023

fix

138e5bc

ReToCode approved these changes Nov 7, 2023

View reviewed changes

knative-prow bot assigned ReToCode Nov 7, 2023

knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Nov 7, 2023

knative-prow bot merged commit 20aa33b into knative:main Nov 7, 2023
16 checks passed

skonto mentioned this pull request Dec 1, 2023

Potentially outdated documentation? #5736

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Finalize pvc support #5651

Finalize pvc support #5651

skonto commented Aug 2, 2023

netlify bot commented Aug 2, 2023 •

edited

Loading

ReToCode Aug 3, 2023 •

edited

Loading

skonto Aug 4, 2023 •

edited

Loading

ReToCode Aug 3, 2023

skonto Aug 4, 2023

ReToCode Aug 4, 2023

skonto Aug 4, 2023 •

edited

Loading

ReToCode Aug 3, 2023

skonto Aug 4, 2023

ReToCode Aug 4, 2023

skonto Aug 4, 2023 •

edited

Loading

skonto commented Oct 3, 2023

ReToCode Oct 3, 2023

skonto Nov 6, 2023 •

edited

Loading

skonto commented Nov 6, 2023

ReToCode left a comment

knative-prow bot commented Nov 7, 2023


		## Prerequisites

		Before you can configure PVCs for a Service, this feature must be enabled in the `KnativeServing` ConfigMap:

Finalize pvc support #5651

Finalize pvc support #5651

Conversation

skonto commented Aug 2, 2023

Proposed Changes

netlify bot commented Aug 2, 2023 • edited Loading

✅ Deploy Preview for knative ready!

ReToCode Aug 3, 2023 • edited Loading

Choose a reason for hiding this comment

skonto Aug 4, 2023 • edited Loading

Choose a reason for hiding this comment

ReToCode Aug 3, 2023

Choose a reason for hiding this comment

skonto Aug 4, 2023

Choose a reason for hiding this comment

ReToCode Aug 4, 2023

Choose a reason for hiding this comment

skonto Aug 4, 2023 • edited Loading

Choose a reason for hiding this comment

ReToCode Aug 3, 2023

Choose a reason for hiding this comment

skonto Aug 4, 2023

Choose a reason for hiding this comment

ReToCode Aug 4, 2023

Choose a reason for hiding this comment

skonto Aug 4, 2023 • edited Loading

Choose a reason for hiding this comment

skonto commented Oct 3, 2023

ReToCode Oct 3, 2023

Choose a reason for hiding this comment

skonto Nov 6, 2023 • edited Loading

Choose a reason for hiding this comment

skonto commented Nov 6, 2023

ReToCode left a comment

Choose a reason for hiding this comment

knative-prow bot commented Nov 7, 2023

netlify bot commented Aug 2, 2023 •

edited

Loading

ReToCode Aug 3, 2023 •

edited

Loading

skonto Aug 4, 2023 •

edited

Loading

skonto Aug 4, 2023 •

edited

Loading

skonto Aug 4, 2023 •

edited

Loading

skonto Nov 6, 2023 •

edited

Loading