Add tests for custom rbd export-diff command #102
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
llamerada-jp
changed the title
llamerada-jp
force-pushed
the
rbd-export-diff-test
branch
14 times, most recently
from
f0bfef2 to
93f1358
Compare
llamerada-jp
force-pushed
the
rbd-export-diff-test
branch
10 times, most recently
from
e7470c2 to
b256928
Compare
llamerada-jp
force-pushed
the
rbd-export-diff-test
branch
5 times, most recently
from
9c3b1d3 to
1a230e8
Compare
llamerada-jp
force-pushed
the
rbd-export-diff-test
branch
2 times, most recently
from
cf7706a to
9366cd3
Compare
llamerada-jp
force-pushed
the
rbd-export-diff-test
branch
2 times, most recently
from
45ce7b8 to
fb019a9
Compare
satoru-takeuchi
requested review from
ushitora-anqou
and removed request for
satoru-takeuchi
Signed-off-by: Yuji Ito <[email protected]>
llamerada-jp
force-pushed
the
rbd-export-diff-test
branch
from
fb019a9 to
2a1a5b1
Compare
ushitora-anqou
requested changes
Feb 21, 2025
ceph/test/suite_test.go
Outdated
|
|
||
| func TestMTest(t *testing.T) { | ||
| if os.Getenv("CEPH_CMD_TEST") == "" { | ||
| t.Skip("Run under ceph/") |
There was a problem hiding this comment.
Can we make the test fail here? I'd like to avoid using t.Skip because I remember topolvm/topolvm#1005.
|
|
||
| description: "(181) not specify snapshot name", | ||
| exportArgs: []string{ | ||
| "--read-offset", "0", |
There was a problem hiding this comment.
This value should be >0 according to the test spec?
ceph/test/regression_test.go
Outdated
|
|
||
| // creating snapshots | ||
| // snapshots[0] and snapshots[1] have diff with the image | ||
| // snapshots[2] and snapshots[2] has no diff with the image |
There was a problem hiding this comment.
Typo?
Suggested change
| // snapshots[2] and snapshots[2] has no diff with the image | |
| // snapshots[2] and snapshots[3] has no diff with the image |
ceph/test/regression_test.go
Outdated
| Expect(err).NotTo(HaveOccurred()) | ||
| } | ||
| // crate snapshot[3] with the same data as snapshot[2] | ||
| // random file snapshot[3] is not exist |
There was a problem hiding this comment.
Suggested change
| // random file snapshot[3] is not exist | |
| // random file named snapshot[3] does not exist |
| expectedDataName: t.snapshots[0], | ||
| exportArgs: []string{ | ||
| "-p", t.poolName, | ||
| fmt.Sprintf("%s@%s", t.srcImageName, t.snapshots[0]), |
There was a problem hiding this comment.
We may need to check pool/image@snap format, too.
ceph/test/cluster/block.go
Outdated
|
|
||
| args := []string{path.Join(workDir, filename), path.Join(workDir, workFilename)} | ||
| log.Printf("📂 diff %s", strings.Join(args, " ")) | ||
| _, err := exec.Command("diff", args...).CombinedOutput() |
There was a problem hiding this comment.
It may be appropriate to use cmp(1) rather than diff(1) because the files are binary.
ceph/test/cluster/kubectl.go
Outdated
Comment on lines
38
to
55
| func Kubectl(args ...string) ([]byte, error) { | ||
| if len(kubectlCmd) == 0 { | ||
| return nil, fmt.Errorf("KUBECTL environment variable should be set") | ||
| } | ||
|
|
||
| icon := "⚓" | ||
| if cephMatcher.MatchString(strings.Join(args, " ")) { | ||
| icon = "🐙" | ||
| } | ||
| log.Printf("%s kubectl %s", icon, strings.Join(args, " ")) | ||
| var stdout bytes.Buffer | ||
| command := exec.Command(kubectlCmd, args...) | ||
| command.Stdout = &stdout | ||
| command.Stderr = os.Stderr | ||
|
|
||
| err := command.Run() | ||
| return stdout.Bytes(), err | ||
| } |
There was a problem hiding this comment.
Can we implement Kubectl by using KubectlWithInput as follows?
Suggested change
| func Kubectl(args ...string) ([]byte, error) { | |
| if len(kubectlCmd) == 0 { | |
| return nil, fmt.Errorf("KUBECTL environment variable should be set") | |
| } | |
| icon := "⚓" | |
| if cephMatcher.MatchString(strings.Join(args, " ")) { | |
| icon = "🐙" | |
| } | |
| log.Printf("%s kubectl %s", icon, strings.Join(args, " ")) | |
| var stdout bytes.Buffer | |
| command := exec.Command(kubectlCmd, args...) | |
| command.Stdout = &stdout | |
| command.Stderr = os.Stderr | |
| err := command.Run() | |
| return stdout.Bytes(), err | |
| } | |
| func Kubectl(args ...string) ([]byte, error) { | |
| return KubectlWithInput(nil, args...) | |
| } |
| } | ||
| if string(after) != string(before) { | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Should we insert some sleeps for reproducibility?
ceph/test/cluster/kubectl.go
Outdated
| } | ||
|
|
||
| func CleanupGlobal() error { | ||
| SCs, err := GetObjectList[storagev1.StorageClassList]("sc", "") |
There was a problem hiding this comment.
Suggested change
| SCs, err := GetObjectList[storagev1.StorageClassList]("sc", "") | |
| scs, err := GetObjectList[storagev1.StorageClassList]("sc", "") |
| ) | ||
|
|
||
| func Rbd(args ...string) ([]byte, error) { | ||
| return Kubectl(append([]string{"exec", "-n", ROOK_NAMESPACE, "deploy/rook-ceph-tools", "--", "rbd"}, args...)...) |
There was a problem hiding this comment.
Can we write this line as follows?
Suggested change
| return Kubectl(append([]string{"exec", "-n", ROOK_NAMESPACE, "deploy/rook-ceph-tools", "--", "rbd"}, args...)...) | |
| return Kubectl("exec", "-n", ROOK_NAMESPACE, "deploy/rook-ceph-tools", "--", "rbd", args...) |
There was a problem hiding this comment.
I tried it, but couldn't.
Signed-off-by: Yuji Ito <[email protected]>
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.
No description provided.