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.

Sign up for GitHub

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

Jump to bottom

Add longhorn storage test #6445

Merged
merged 17 commits into from
Apr 28, 2023
Merged

Add longhorn storage test #6445

merged 17 commits into from
Apr 28, 2023

Conversation

brooksn
Copy link
Member

@brooksn brooksn commented Nov 4, 2022

Proposed Changes

Adds a basic test for the Longhorn setup steps in the k3s documentation:
https://docs.k3s.io/storage#setting-up-longhorn

Types of Changes

Verification

Testing

Linked Issues

#6061

User-Facing Change

Further Comments

@brooksn brooksn requested a review from a team as a code owner November 4, 2022 17:05
@brooksn brooksn self-assigned this Nov 4, 2022
brandond
brandond reviewed Nov 4, 2022
View reviewed changes
Copy link
Member

@brandond brandond left a comment

Choose a reason for hiding this comment

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

one nit, LGTM otherwise!

Do we want to also test the CNI snapshotter functionality? That requires installing some extra bits I guess...

tests/integration/longhorn/longhorn_int_test.go Outdated Show resolved Hide resolved
dereknola
dereknola requested changes Nov 4, 2022
View reviewed changes
tests/integration/longhorn/longhorn_int_test.go Outdated Show resolved Hide resolved
tests/integration/longhorn/longhorn_int_test.go Outdated Show resolved Hide resolved
tests/integration/longhorn/longhorn_int_test.go Outdated Show resolved Hide resolved
tests/integration/longhorn/longhorn_int_test.go Outdated Show resolved Hide resolved
tests/integration/longhorn/longhorn_int_test.go Outdated Show resolved Hide resolved
@dereknola
Copy link
Member

Looking at the CI failure, you may need to A) bump the timeout time on some of your checks or B) reduce the request size of the storage. The GH runners don't have that much space on them.

@matttrach
Copy link
Contributor

just to short circuit some troubleshooting attempts the following error was thrown:

• [FAILED] [30.386 seconds]
longhorn
/home/runner/work/k3s/k3s/tests/integration/longhorn/longhorn_int_test.go:35
  when persistent volume claim is created
  /home/runner/work/k3s/k3s/tests/integration/longhorn/longhorn_int_test.go:84
    [It] creates a pod with the pvc
    /home/runner/work/k3s/k3s/tests/integration/longhorn/longhorn_int_test.go:111

  Timed out after 30.004s.
  Expected success, but got an error:
      <*errors.errorString | 0xc0003b2370>: {
          s: "pod volume-test not running",
      }
      pod volume-test not running
  In [It] at: /home/runner/work/k3s/k3s/tests/integration/longhorn/longhorn_int_test.go:128

@matttrach
Copy link
Contributor

The pvc was successfully generated in the previous test:

• [SLOW TEST] [11.487 seconds]
longhorn when persistent volume claim is created creates the pv and pvc
/home/runner/work/k3s/k3s/tests/integration/longhorn/longhorn_int_test.go:85

@matttrach
Copy link
Contributor

Looking at the debug log it appears to have timed out attempting to ssh into the container:

SSH: ssh [email protected]
Web shell: https://tmate.io/t/3spcPn8DJjWWr9d6YACQAky9J

It says "web shell", does that mean it is attempting on port 80?
That is the only port exposed by the pod, I believe.

@brooksn brooksn force-pushed the 6061-expand-integration-test-for-longhorn-storage branch from 8b5db25 to ba559de Compare March 1, 2023 17:59
brooksn added 5 commits March 1, 2023 11:03
@brooksn
add basic longhorn storage test
291f20d 
Signed-off-by: Brooks Newberry <[email protected]>
@brooksn
update longhorn to v1.4.0
5b82e65 
Signed-off-by: Brooks Newberry <[email protected]>
@brooksn
move k8s client methods to testutils
4627f21 
Signed-off-by: Brooks Newberry <[email protected]>
@brooksn
remove unused SpecContext
f0e90f8 
Signed-off-by: Brooks Newberry <[email protected]>
@brooksn
change longhorn int test package name
471f7f3 
Signed-off-by: Brooks Newberry <[email protected]>
@brooksn brooksn force-pushed the 6061-expand-integration-test-for-longhorn-storage branch from c735d2d to 471f7f3 Compare March 1, 2023 19:04
brooksn added 9 commits March 2, 2023 10:13
@brooksn
remove println
72b9b20 
Signed-off-by: Brooks Newberry <[email protected]>
@brooksn
increase timeout
b2ff4de 
Signed-off-by: Brooks Newberry <[email protected]>
@brooksn
dump logs
d310bda 
Signed-off-by: Brooks Newberry <[email protected]>
@brooksn
reduce available storage percent
bde579c 
Signed-off-by: Brooks Newberry <[email protected]>
@brooksn
fix spelling
d934f7b 
Signed-off-by: Brooks Newberry <[email protected]>
@brooksn
fix duplicate err check line
df8abab 
Signed-off-by: Brooks Newberry <[email protected]>
@brooksn
fix late err check
3f0d4f3 
Signed-off-by: Brooks Newberry <[email protected]>
@brooksn
fix test case description
939dfd8 
Signed-off-by: Brooks Newberry <[email protected]>
@brooksn
fix test lock release
ba170e2 
Signed-off-by: Brooks Newberry <[email protected]>
dereknola
dereknola requested changes Mar 17, 2023
View reviewed changes
tests/integration/longhorn/longhorn_int_test.go
return nil
}, "60s", "5s").Should(Succeed())
})
})
Copy link
Member

@dereknola dereknola Mar 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Can you add a section that tests deleting the longhorn pvc? Also doubles as cleanup for test.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, it looks like this test cleans up the storage replicas.

brooksn added 2 commits April 4, 2023 21:47
@brooksn
simplify returns
859e1f5 
Signed-off-by: Brooks Newberry <[email protected]>
@brooksn
check errors before results
fb8c0c6 
Signed-off-by: Brooks Newberry <[email protected]>
@brooksn
add cleanup test
b441138 
Signed-off-by: Brooks Newberry <[email protected]>
@brooksn brooksn requested review from dereknola and briandowns April 5, 2023 20:37
@brooksn brooksn merged commit bbda54b into k3s-io:master Apr 28, 2023
@brooksn brooksn deleted the 6061-expand-integration-test-for-longhorn-storage branch April 28, 2023 18:35
osodracnai pushed a commit to osodracnai/k3s that referenced this pull request May 8, 2023
@brooksn @osodracnai
Add longhorn storage test (k3s-io#6445)
8741dc6 
Signed-off-by: Brooks Newberry <[email protected]>
(cherry picked from commit bbda54b)
@osodracnai osodracnai mentioned this pull request May 8, 2023
Closed
osodracnai pushed a commit to osodracnai/k3s that referenced this pull request May 8, 2023
@brooksn @osodracnai
Add longhorn storage test (k3s-io#6445)
7340491 
Signed-off-by: Brooks Newberry <[email protected]>
(cherry picked from commit bbda54b)
Signed-off-by: Ian Cardoso <[email protected]>
osodracnai pushed a commit to osodracnai/k3s that referenced this pull request May 8, 2023
@brooksn @osodracnai
Add longhorn storage test (k3s-io#6445)
efd7b6f 
Signed-off-by: Brooks Newberry <[email protected]>
osodracnai pushed a commit to osodracnai/k3s that referenced this pull request May 9, 2023
@brooksn @osodracnai
Add longhorn storage test (k3s-io#6445)
9bb2dc2 
Signed-off-by: Brooks Newberry <[email protected]>
Signed-off-by: Ian Cardoso <[email protected]>
@osodracnai osodracnai mentioned this pull request May 9, 2023
Merged
osodracnai pushed a commit to osodracnai/k3s that referenced this pull request May 9, 2023
@brooksn @osodracnai
Add longhorn storage test (k3s-io#6445)
52930bd 
Signed-off-by: Brooks Newberry <[email protected]>
(cherry picked from commit bbda54b)
@osodracnai osodracnai mentioned this pull request May 9, 2023
Merged
osodracnai pushed a commit to osodracnai/k3s that referenced this pull request May 9, 2023
@brooksn @osodracnai
Add longhorn storage test (k3s-io#6445)
910dfdb 
Signed-off-by: Brooks Newberry <[email protected]>
(cherry picked from commit bbda54b)
Signed-off-by: Ian Cardoso <[email protected]>
osodracnai pushed a commit to osodracnai/k3s that referenced this pull request May 9, 2023
@brooksn @osodracnai
Add longhorn storage test (k3s-io#6445)
e5e1449 
Signed-off-by: Brooks Newberry <[email protected]>
(cherry picked from commit bbda54b)
@osodracnai osodracnai mentioned this pull request May 9, 2023
Merged
osodracnai pushed a commit to osodracnai/k3s that referenced this pull request May 9, 2023
@brooksn @osodracnai
Add longhorn storage test (k3s-io#6445)
c4ab917 
Signed-off-by: Brooks Newberry <[email protected]>
(cherry picked from commit bbda54b)
Signed-off-by: Ian Cardoso <[email protected]>
osodracnai pushed a commit that referenced this pull request May 9, 2023
@brooksn @osodracnai
Add longhorn storage test (#6445)
9b9fd04 
Signed-off-by: Brooks Newberry <[email protected]>
(cherry picked from commit bbda54b)
Signed-off-by: Ian Cardoso <[email protected]>
osodracnai pushed a commit that referenced this pull request May 9, 2023
@brooksn @osodracnai
Add longhorn storage test (#6445)
a10ef1d 
Signed-off-by: Brooks Newberry <[email protected]>
(cherry picked from commit bbda54b)
Signed-off-by: Ian Cardoso <[email protected]>
osodracnai pushed a commit that referenced this pull request May 9, 2023
@brooksn @osodracnai
Add longhorn storage test (#6445)
21a96c6 
Signed-off-by: Brooks Newberry <[email protected]>
Signed-off-by: Ian Cardoso <[email protected]>
@cguertin14 cguertin14 mentioned this pull request May 31, 2023
Merged
exp0nge
exp0nge reviewed Apr 20, 2024
View reviewed changes
tests/integration/longhorn/longhorn_int_test.go

var _ = BeforeSuite(func() {
if _, err := exec.LookPath("iscsiadm"); err != nil {
Skip("Test needs open-iscsi to be installed")

This comment was marked as off-topic.

Sign in to view

Copy link
Member

Choose a reason for hiding this comment

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

Please open a discussion if you have questions. Year old PRs are not the correct place.

@k3s-io k3s-io locked as resolved and limited conversation to collaborators Apr 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@exp0nge exp0nge exp0nge left review comments

@brandond brandond brandond approved these changes

@dereknola dereknola dereknola approved these changes

@briandowns briandowns Awaiting requested review from briandowns

Assignees

@brooksn brooksn

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@brooksn @dereknola @matttrach @brandond @briandowns @exp0nge