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

Recursively add metadata JSON fields to the tree #149

Merged
merged 1 commit into from
Oct 23, 2024

Conversation

rst0git
Copy link
Member

@rst0git rst0git commented Oct 23, 2024

Some annotations contain a JSON string, and this string may include nested arrays and maps. This pull request updates addPodInfoToTree() to recursively add these fields to the output tree using pretty print.

Before:

    ├── kubectl.kubernetes.io/last-applied-configuration
    │   ├── metadata: map[annotations:map[] name:counter-pod namespace:default]
    │   ├── spec: map[containers:[map[args:[-c i=0; while true; do echo $i; i=$(expr $i + 1); sleep 1; done] command:[sh] image:busybox:latest name:counter-container]]]

After:

    ├── kubectl.kubernetes.io/last-applied-configuration
    │   ├── apiVersion: v1
    │   ├── kind: Pod
    │   ├── metadata:
    │   │   ├── annotations:
    │   │   ├── name: counter-pod
    │   │   └── namespace: default
    │   └── spec:
    │       └── containers: 
    │           └── [0]:
    │               ├── args: 
    │               │   ├── [0]: -c
    │               │   └── [1]: i=0; while true; do echo $i; i=$(expr $i + 1); sleep 1; done
    │               ├── command: 
    │               │   └── [0]: sh
    │               ├── image: busybox:latest
    │               └── name: counter-container

@rst0git rst0git requested a review from adrianreber October 23, 2024 08:58
@codecov-commenter
Copy link

codecov-commenter commented Oct 23, 2024

Codecov Report

Attention: Patch coverage is 0% with 28 lines in your changes missing coverage. Please review.

Project coverage is 76.42%. Comparing base (c770752) to head (2e305b9).

Files with missing lines Patch % Lines
internal/tree.go 0.00% 28 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #149      +/-   ##
==========================================
- Coverage   77.76%   76.42%   -1.35%     
==========================================
  Files          11       11              
  Lines        1417     1442      +25     
==========================================
  Hits         1102     1102              
- Misses        241      266      +25     
  Partials       74       74              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

github-actions bot commented Oct 23, 2024

Test Results

60 tests  ±0   60 ✅ ±0   1s ⏱️ ±0s
 1 suites ±0    0 💤 ±0 
 1 files   ±0    0 ❌ ±0 

Results for commit 2e305b9. ± Comparison against base commit c770752.

♻️ This comment has been updated with latest results.

@adrianreber
Copy link
Member

Nice, I was also thinking about something like that. I am not sure about:

├── args: [
...
└── ]

That look a bit strange. Could this just be left away? Or why do you print it? Any special reason it might be useful?

@snprajwal
Copy link
Member

+1, I think we can just leave the braces out, similar to metadata:

Some annotations contain a JSON string. This string may include
nested arrays and maps. This patch updates addPodInfoToTree()
to recursively add these fields to the output tree using pretty print.

Before:

        ├── kubectl.kubernetes.io/last-applied-configuration
        │   ├── metadata: map[annotations:map[] name:counter-pod namespace:default]
        │   ├── spec: map[containers:[map[args:[-c i=0; while true; do echo $i; i=$(expr $i + 1); sleep 1; done] command:[sh] image:busybox:latest name:counter-container]]]

After:

        ├── kubectl.kubernetes.io/last-applied-configuration
        │   ├── apiVersion: v1
        │   ├── kind: Pod
        │   ├── metadata:
        │   │   ├── annotations:
        │   │   ├── name: counter-pod
        │   │   └── namespace: default
        │   └── spec:
        │       └── containers:
        │           └── [0]:
        │               ├── args:
        │               │   ├── [0]: -c
        │               │   └── [1]: i=0; while true; do echo $i; i=$(expr $i + 1); sleep 1; done
        │               ├── command:
        │               │   └── [0]: sh
        │               ├── image: busybox:latest
        │               └── name: counter-container

Signed-off-by: Radostin Stoyanov <[email protected]>
@rst0git rst0git force-pushed the 2024-10-23-metadata branch from 6632ed1 to 2e305b9 Compare October 23, 2024 09:31
@rst0git
Copy link
Member Author

rst0git commented Oct 23, 2024

Thanks, I've updated the pull request to remove the brackets in the output.

@adrianreber adrianreber merged commit 9a448d5 into checkpoint-restore:main Oct 23, 2024
10 checks passed
@rst0git rst0git deleted the 2024-10-23-metadata branch October 23, 2024 10:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants