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

volume-requested-topology annotation should be YAML string for better readability #3109

Open
aruneshpa opened this issue Nov 15, 2024 · 4 comments
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@aruneshpa
Copy link

Is this a BUG REPORT or FEATURE REQUEST?:
/kind bug

What happened:
The current way to specify the list of zones where a volume would be available is to specify a JSON blob as a value of the volume-requested-topology annotation. While this works, this quickly becomes unreadable for any human interaction and consumption. A cleaner way would be to use YAML strings. Here a comparison of both current and the proposed approach:

Consider a scenario where a PVC is being made available in 5 zones (for illustration purpose). The value of the annotation starts to becomes unreadable.

apiVersion: v1
kind: PersistentVolumeClaim
metadata:
  annotations:
    csi.vsphere.volume-requested-topology: '[{"topology.kubernetes.io/zone":"zone-1"}, {"topology.kubernetes.io/zone":"zone-2"}, {"topology.kubernetes.io/zone":"zone-3"}, {"topology.kubernetes.io/zone":"zone-4"}, {"topology.kubernetes.io/zone":"zone-5"}]'

The same PVC will look like this using the proposed YAML string approach:

apiVersion: v1
kind: PersistentVolumeClaim
metadata:
  annotations:
    csi.vsphere.volume-requested-topology: |
      - "topology.kubernetes.io/zone":"zone-1"
      - "topology.kubernetes.io/zone":"zone-2"
      - "topology.kubernetes.io/zone":"zone-3"
      - "topology.kubernetes.io/zone":"zone-4"
      - "topology.kubernetes.io/zone":"zone-5"

YAML as a string also has an advantage of being able to identify any mistakes while crafting the object (essential for non automation/machine users).

What you expected to happen:
N/A

How to reproduce it (as minimally and precisely as possible):
Create a multi zone PVC and observe the JSON blob in the annotation.

Anything else we need to know?:
N/A

Skipping the env section since this is present on main.

@k8s-ci-robot k8s-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Nov 15, 2024
@aruneshpa
Copy link
Author

cc @SandeepPissay , @akutz

@akutz
Copy link
Contributor

akutz commented Nov 15, 2024

Please note the use of the | character after the annotation key. This indicates a multi-line YAML string. I validated this works for annotations by creating a ConfigMap with one. Importantly the info is also returned in this format as well.

@akutz
Copy link
Contributor

akutz commented Nov 18, 2024

A demonstration

For what it is worth, here are steps to reproduce the multi-line YAML string as the annotation value:

  1. Create a Kind cluster:

    kind create cluster --name multi-line-yaml

    The above command should result in output similar to the following:

    using podman due to KIND_EXPERIMENTAL_PROVIDER
    enabling experimental podman provider
    Creating cluster "multi-line-yaml" ...
     ✓ Ensuring node image (kindest/node:v1.30.0) 🖼 
     ✓ Preparing nodes 📦  
     ✓ Writing configuration 📜 
     ✓ Starting control-plane 🕹️ 
     ✓ Installing CNI 🔌 
     ✓ Installing StorageClass 💾 
    Set kubectl context to "kind-multi-line-yaml"
    You can now use your cluster with:
    
    kubectl cluster-info --context kind-multi-line-yaml
    
    Thanks for using kind! 😊
  2. Tell kubectl to use the Kind cluster's context:

    kubectl config use-context kind-multi-line-yaml

    The above command should emit the following:

    Switched to context "kind-multi-line-yaml".
  3. Create a ConfigMap with the annotation csi.vsphere.volume-requested-topology set to a multi-line YAML string:


    🌟 Please note

    • The | character that leads the value in the csi.vsphere.volume-requested-topology annotation indicates the value is a block-style multi-line YAML string.
    • The --server-side flag in the below command prevents kubectl from adding the annotation kubectl.kubernetes.io/last-applied-configuration to the object. This is not required for this demo, but it does make the object less wordy and easier to understand.

    kubectl -n default apply --server-side -f - <<EOF
    apiVersion: v1
    kind: ConfigMap
    metadata:
      name: my-configmap-1
      annotations:
        csi.vsphere.volume-requested-topology: |
          - topology.kubernetes.io/zone: zone-2
          - topology.kubernetes.io/zone: zone-3
    data:
      hello: world
    EOF

    The above command will emit the following output:

    configmap/my-configmap-1 serverside-applied
  4. Get the ConfigMap from the cluster and print it as YAML:

    kubectl -n default get configmap my-configmap-1 -oyaml

    The above command will print the object, retaining the multi-line string format for the annotation csi.vsphere.volume-requested-topology:

    apiVersion: v1
    data:
      hello: world
    kind: ConfigMap
    metadata:
      annotations:
        csi.vsphere.volume-requested-topology: |
          - topology.kubernetes.io/zone: zone-2
          - topology.kubernetes.io/zone: zone-3
      creationTimestamp: "2024-11-18T13:27:38Z"
      name: my-configmap-1
      namespace: default
      resourceVersion: "1143"
      uid: 518d4540-dff7-42ec-84f1-29a01619a18d
  5. Get just the value of the annotation csi.vsphere.volume-requested-topology:

    kubectl -n default get configmap my-configmap-1 -ojsonpath='{.metadata.annotations.csi\.vsphere\.volume-requested-topology}'

    The result should be identical to:

    - topology.kubernetes.io/zone: zone-2
    - topology.kubernetes.io/zone: zone-3

Compatibility

YAML is a superset of JSON and thus it is possible to convert between the two. For example, the above object could also support setting JSON values in the annotation:


🌟 Please note

  • The following steps require the command-line tool yq, as it is used to convert YAML to JSON and JSON to YAML.

  1. Update the object with a JSON value for the annotation by using the yq command to convert the YAML value to JSON:
    kubectl -n default apply --server-side -f - <<EOF
    apiVersion: v1
    kind: ConfigMap
    metadata:
      name: my-configmap-1
      annotations:
        csi.vsphere.volume-requested-topology: '$(kubectl -n default get configmap my-configmap-1 -ojsonpath='{.metadata.annotations.csi\.vsphere\.volume-requested-topology}' | yq -pyaml -ojson -I0)'
    EOF
    The above command will emit the following output:
    configmap/my-configmap-1 serverside-applied
  2. Get just the value of the annotation csi.vsphere.volume-requested-topology:
    kubectl -n default get configmap my-configmap-1 -ojsonpath='{.metadata.annotations.csi\.vsphere\.volume-requested-topology}'
    The result should be identical to:
    [{"topology.kubernetes.io/zone":"zone-2"}, {"topology.kubernetes.io/zone":"zone-3"}]
  3. Get the annotation again, but this time use the yq command to translate the JSON to YAML:
    kubectl -n default get configmap my-configmap-1 -ojsonpath='{.metadata.annotations.csi\.vsphere\.volume-requested-topology}' | yq -pjson -oyaml
    The value stored as JSON will be printed instead as YAML:
    - topology.kubernetes.io/zone: zone-2
    - topology.kubernetes.io/zone: zone-3

This example illustrates how a mutation webhook could be used to translate JSON --> YAML on any non-empty values for the annotation csi.vsphere.volume-requested-topology on incoming PersistentVolumeClaim resources.

This way users can specify both JSON and YAML for input, but the output can always be YAML for improved readability.

@divyenpatel
Copy link
Member

Thanks @aruneshpa @akutz for detailed feature request.
I am creating internal jira to get this prioritized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

No branches or pull requests

4 participants