Skip to content

Check CSI sidecar supported versions#227

Merged
k8s-ci-robot merged 4 commits intokubernetes-csi:masterfrom
coulof:check-sidecar-supported-versions
Jul 24, 2023
Merged

Check CSI sidecar supported versions#227
k8s-ci-robot merged 4 commits intokubernetes-csi:masterfrom
coulof:check-sidecar-supported-versions

Conversation

@coulof
Copy link
Copy Markdown
Contributor

@coulof coulof commented May 16, 2023

That script is a helper to update the tables from : https://kubernetes-csi.github.io/docs/sidecar-containers.html

The next goal is to generate the tables from the release pages but... we have to agree upon the metadata and fix the typos :-) (cf. https://github.com/kubernetes-csi/external-attacher/releases with docker pull docker pull registry.k8s.io/sig-storage/csi-attacher:v4.3.0)

@k8s-ci-robot k8s-ci-robot added the do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. label May 16, 2023
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 16, 2023
@coulof
Copy link
Copy Markdown
Contributor Author

coulof commented May 16, 2023

/release-note-none

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels May 16, 2023
Copy link
Copy Markdown
Contributor

@ggriffiths ggriffiths left a comment

Choose a reason for hiding this comment

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

/lgtm, looks good to me! Thanks for adding this!

@ggriffiths
Copy link
Copy Markdown
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 30, 2023
@coulof
Copy link
Copy Markdown
Contributor Author

coulof commented Jun 30, 2023

Thanks @ggriffiths.
I think this needs a /approve too ;-)

Copy link
Copy Markdown
Member

@mauriciopoppe mauriciopoppe left a comment

Choose a reason for hiding this comment

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

Thanks for taking the time to write this tool. If there's tooling outside the usual bash scripts/makefile scripts I think all of our repos are written in go so it'd make sense to write tooling in go too.

from collections import defaultdict
import subprocess
import shutil
from dateutil.relativedelta import relativedelta
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If this is an external dependency I think it needs to have a pip install command somewhere but it's a dev only tool so I think it's good.

Also consider that this file is going to be synced to all the CSI repos.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

collections is a built-in ; dateutil is an external dependency.

Next time I work on it I will replace it with just built-in ;-)

@ggriffiths
Copy link
Copy Markdown
Contributor

Thanks @ggriffiths. I think this needs a /approve too ;-)

Yes, you'll need @jsafrane for an approval review. I can only provide LGTM :)

@ggriffiths
Copy link
Copy Markdown
Contributor

/assign @jsafrane

@ggriffiths
Copy link
Copy Markdown
Contributor

Thanks for taking the time to write this tool. If there's tooling outside the usual bash scripts/makefile scripts I think all of our repos are written in go so it'd make sense to write tooling in go too.

I think there are some K8s scripts out there written in a Python. It's a much better language than bash or go for this use case.

@jsafrane
Copy link
Copy Markdown
Contributor

/approve
sorry it took so long

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: coulof, ggriffiths, jsafrane

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 24, 2023
@k8s-ci-robot k8s-ci-robot merged commit c10b678 into kubernetes-csi:master Jul 24, 2023
@@ -0,0 +1,170 @@
# Copyright 2023 The Kubernetes Authors.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Some repos have codespell enabled as a github action which failed when this change was synced to the other repos, could you please review these errors?

Resulting CLI options  --check-filenames --skip ./.git,./.github/workflows/codespell.yml,.git,*.png,*.jpg,*.svg,*.sum,./vendor,go.sum,./release-tools/prow.sh
3
Error: ./release-tools/contrib/get_supported_version_csi-sidecar.py:35: ouputs ==> outputs
Error: ./release-tools/contrib/get_supported_version_csi-sidecar.py:101: relase ==> release

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

addressed in #229

@coulof coulof deleted the check-sidecar-supported-versions branch March 12, 2025 10:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants