cephfs: addressed few todo #4231
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
this commit adds the validation for encryption value as false, and sets the type as none Signed-off-by: riya-singhal31 <[email protected]>
|
/test ci/centos/mini-e2e-helm/k8s-1.27 |
Madhu-1
reviewed
Nov 6, 2023
internal/cephfs/core/metadata.go
Outdated
| if err != nil && !strings.Contains(err.Error(), "No such file or directory") { | ||
| return fmt.Errorf("failed to unset metadata key %q on subvolume %v: %w", key, s, err) | ||
| if err != nil { | ||
| if errors.As(err, &errno) && errno == syscall.ENOENT { |
There was a problem hiding this comment.
@riya-singhal31 please add an E2E to ensure this working as expected or please test this manually by removing the metadata in the backend.
There was a problem hiding this comment.
@Madhu-1 I'm replacing this errno with the actual errors that are returned by go-ceph, like for ENOENT they have "ErrNotExist "https://github.com/ceph/go-ceph/blob/master/rbd/errors.go#L77 .
But for cephFS, they have the error as private https://github.com/ceph/go-ceph/blob/master/cephfs/errors.go#L64, so sending a patch there to make it a public and then will modify this PR to direct compare the errors using errors.Is(), that might look more better.
internal/rbd/rbd_util.go
Outdated
| // TODO: replace string comparison with errno. | ||
| if err != nil && !strings.Contains(err.Error(), "No such file or directory") { | ||
| return fmt.Errorf("failed to unset metadata key %q on %q: %w", key, rv, err) | ||
| if err != nil { |
There was a problem hiding this comment.
please add an E2E to ensure this working as expected or please test this manually by removing the metadata in the backend.
|
This pull request now has conflicts with the target branch. Could you please resolve conflicts and force push the corrected changes? 🙏 |
riya-singhal31
changed the title
This commit eliminates the code for protecting and unprotecting snapshots, as the functionality to protect and unprotect snapshots is being deprecated. Signed-off-by: Praveen M <[email protected]>
Signed-off-by: Praveen M <[email protected]>
This commit removes the deprecated grpc related code from cephcsi. fixes: #4122 Signed-off-by: Madhu Rajanna <[email protected]>
re-arrange the struct members to fix below lint issue ``` struct of size 336 bytes could be of size 328 bytes ``` Signed-off-by: Madhu Rajanna <[email protected]>
Keeping track of changes between releases and fetching that information during release is difficult, Adding a doc to keep track of the changes between major releases which helps during release. Signed-off-by: Madhu Rajanna <[email protected]>
tickgit.com identifies `XXX` as label for TODO's. There is no need to list the `digest` hash examples in the TODO list, so replace the `XXX` and `YYY` examples with `XYZ` and `ZYX`. Signed-off-by: Niels de Vos <[email protected]>
Signed-off-by: Niels de Vos <[email protected]>
Signed-off-by: Niels de Vos <[email protected]>
The HealthChecker is configured to use the Staging path pf the volume, with a `.csi/` subdirectory. In the future this directory could be a directory that is not under the Published directory. Fixes: #4219 Signed-off-by: Niels de Vos <[email protected]>
When FilesystemNodeGetVolumeStats() succeeds, the volume must be healthy. This can be included in the VolumeCondition CSI message by default. Checks that detect an abnormal VolumeCondition should prevent calling FilesystemNodeGetVolumeStats() as it is possible that the function will hang. Signed-off-by: Niels de Vos <[email protected]>
update the e2e code to add multiple labels to the node at a time. fixes: #4146 Signed-off-by: Madhu Rajanna <[email protected]>
update the e2e code to remove multiple labels to the node at a time. Signed-off-by: Madhu Rajanna <[email protected]>
As we are using latest version of go-ceph which doesnt require any special tags for the API's cephcsi is consuming. Signed-off-by: Madhu Rajanna <[email protected]>
remove RadosNamespace from the configmap main section as its already added to the rbd section Signed-off-by: Madhu Rajanna <[email protected]>
remove support for deprecated rbdImageRequiresEncryption case. Signed-off-by: Madhu Rajanna <[email protected]>
this commit updates the go-ceph version to use the latest(master) branch so as to be able to import cephfs. Signed-off-by: Riya Singhal <[email protected]>
|
This pull request now has conflicts with the target branch. Could you please resolve conflicts and force push the corrected changes? 🙏 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
[1]This commit updates the go-ceph version to use the latest branch
[2] This commit addressed few todo, replacing string comparison with ErrNotExist code of cephFS and rbd