-
Notifications
You must be signed in to change notification settings - Fork 553
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
deploy: API for CSI Config Struct #4278
deploy: API for CSI Config Struct #4278
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You'll need to do the following too:
- remove the file from the
deploy/
directory - add the generation to
tools/yamlgen/main.go
- add the make target to
deploy/Makefile
- run
make deploy
from the root of the project - commit the newly generated file that you removed in step 1
18a147c
to
8901d3f
Compare
Done, |
@@ -12,4 +12,4 @@ metadata: | |||
name: "ceph-csi-config" | |||
data: | |||
config.json: |- | |||
[] | |||
[{Dmeo [] { } { } {} {true [a b]}}] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dmeo?
This format does not look valid, so the generation isn't setup correctly. I don't know where this comes from, but the values need to be set so that the file can be applied directly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops!, Just for testing I had modified in config map,
this got added, didn't notice it
Apologies
@@ -7,3 +7,4 @@ spec: | |||
attachRequired: false | |||
podInfoOnMount: false | |||
fsGroupPolicy: File | |||
seLinuxMount: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A modification to this file should result in updated generated files under deploy/
too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seLinuxMount: true
was added under deploy/
files here - c0201e4
So, generated files match with the files under deploy/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, interesting! In that case we need to run make deploy
in the CI to check that all modified files are included in a PR. When you run make deploy
, the seLinuxMount: true
option should be removed from the deploy/
directory without your change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well, sortof. See this:
$ make -C tools generate-deploy
make: Entering directory '/var/tmp/ceph-csi/tools'
go run yamlgen/main.go
creating "../deploy/scc.yaml"...done!
creating "../deploy/cephfs/kubernetes/csidriver.yaml"...done!
creating "../deploy/cephfs/kubernetes/csi-config-map.yaml"...done!
creating "../deploy/nfs/kubernetes/csidriver.yaml"...done!
creating "../deploy/nfs/kubernetes/csi-config-map.yaml"...done!
creating "../deploy/rbd/kubernetes/csidriver.yaml"...done!
creating "../deploy/rbd/kubernetes/csi-config-map.yaml"...done!
make: Leaving directory '/var/tmp/ceph-csi/tools'
$ git status
HEAD detached at origin/devel
Changes not staged for commit:
(use "git add <file>..." to update what will be committed)
(use "git restore <file>..." to discard changes in working directory)
modified: deploy/cephfs/kubernetes/csidriver.yaml
modified: deploy/nfs/kubernetes/csidriver.yaml
modified: deploy/rbd/kubernetes/csidriver.yaml
no changes added to commit (use "git add" and/or "git commit -a")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, it seems that this is not checked in Ceph-CSI at all now. We need a workflow similar to kubernetes-csi-addons that does this check:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@iPraveenParihar do you want to take this on, and create a PR for that? Otherwise we'll need to create it as an issue so that we won't forget it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
created Issue - #4285
8901d3f
to
39eddc7
Compare
@Mergifyio queue |
✅ The pull request has been merged automaticallyThe pull request has been merged automatically at 878eef8 |
This commit exposes CSI ConfigMap over an API. This will allow projects like Rook to consume CSI configMap directly from Ceph-CSI. Signed-off-by: Praveen M <m.praveen@ibm.com>
39eddc7
to
0b3b2e8
Compare
/test ci/centos/k8s-e2e-external-storage/1.27 |
/test ci/centos/mini-e2e-helm/k8s-1.27 |
/test ci/centos/k8s-e2e-external-storage/1.26 |
/test ci/centos/upgrade-tests-cephfs |
/test ci/centos/mini-e2e/k8s-1.27 |
/test ci/centos/mini-e2e-helm/k8s-1.26 |
/test ci/centos/upgrade-tests-rbd |
/test ci/centos/mini-e2e/k8s-1.26 |
/test ci/centos/k8s-e2e-external-storage/1.28 |
/test ci/centos/mini-e2e-helm/k8s-1.28 |
/test ci/centos/mini-e2e/k8s-1.28 |
Describe what this PR does
Exposing CSI Config Map struct.
Checklist:
guidelines in the developer guide.
updated with breaking and/or notable changes for the next major release.
Show available bot commands
These commands are normally not required, but in case of issues, leave any of
the following bot commands in an otherwise empty comment in this PR:
/retest ci/centos/<job-name>
: retest the<job-name>
after unrelatedfailure (please report the failure too!)