Skip to content

Fix K3S start command#7677

Merged
eddumelendez merged 5 commits intotestcontainers:mainfrom
tgeens:k3s-disable-flag
Oct 24, 2023
Merged

Fix K3S start command#7677
eddumelendez merged 5 commits intotestcontainers:mainfrom
tgeens:k3s-disable-flag

Conversation

@tgeens
Copy link
Copy Markdown
Contributor

@tgeens tgeens commented Oct 17, 2023

This PR fixes #6770 by swapping the deprecated/removed --no-deploy flag with the replacement flag --disable

This change is backwards compatible and tested with a wide range of versions:

  • oldest version succesfully tested: rancher/k3s:v1.17.17-k3s1
  • current version (updated in tests): rancher/k3s:v1.25.14-k3s1

See:

@tgeens tgeens requested a review from a team October 17, 2023 08:33
@eddumelendez
Copy link
Copy Markdown
Member

eddumelendez commented Oct 17, 2023

Thanks for your contribution, @tgeens ! Can we add a test with rancher/k3s:v1.17.17-k3s1 as a proof of being backward compatible, please?

@eddumelendez eddumelendez added this to the next milestone Oct 17, 2023
@eddumelendez eddumelendez changed the title Fixes #6770: k3s >= 1.24 will not start Fix K3S start command Oct 17, 2023
eddumelendez
eddumelendez reviewed Oct 17, 2023
View reviewed changes
Comment thread modules/k3s/src/test/java/org/testcontainers/k3s/Fabric8K3sContainerTest.java Outdated
@tgeens tgeens requested a review from eddumelendez October 18, 2023 09:05
eddumelendez
eddumelendez reviewed Oct 18, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@eddumelendez eddumelendez left a comment

Choose a reason for hiding this comment

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

I've left a few comments/suggestions. After that I'll be merging the PR. Thanks again!.

Comment thread modules/k3s/src/test/java/org/testcontainers/k3s/Fabric8K3sContainerTest.java Outdated
Comment thread modules/k3s/src/test/java/org/testcontainers/k3s/OfficialClientK3sContainerTest.java Outdated
@tgeens tgeens requested a review from eddumelendez October 20, 2023 08:40
eddumelendez
eddumelendez previously approved these changes Oct 20, 2023
View reviewed changes
@eddumelendez
Copy link
Copy Markdown
Member

eddumelendez commented Oct 20, 2023 •
edited
Loading

Hi @tgeens, looks like the wait strategy fails using rancher/k3s:v1.17.17-k3s1. Can you take a look, please?

@github-actions github-actions Bot added the github_actions Pull requests that update Github_actions code label Oct 23, 2023
eddumelendez
eddumelendez requested changes Oct 23, 2023
View reviewed changes
Comment thread .github/workflows/ci.yml Outdated
- '.sdkmanrc'
push:
branches: [ main ]
branches: [ main, k3s-disable-flag ]
Copy link
Copy Markdown
Member

@eddumelendez eddumelendez Oct 23, 2023

Choose a reason for hiding this comment

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

this is not needed

Comment thread modules/k3s/src/main/java/org/testcontainers/k3s/K3sContainer.java Outdated
setWaitStrategy(
new LogMessageWaitStrategy()
.withRegEx(".*Node controller sync successful.*")
.withStartupTimeout(Duration.ofMinutes(5))
Copy link
Copy Markdown
Member

@eddumelendez eddumelendez Oct 23, 2023

Choose a reason for hiding this comment

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

so, 5 min is required? The max should be 1 min for good experience.

Copy link
Copy Markdown
Contributor Author

@tgeens tgeens Oct 23, 2023

Choose a reason for hiding this comment

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

Sorry, this were my own tests, I should have done these experiments on a separate branch, not on this PR.

I'm trying to figure out why local ./gradlew :k3s:test works just fine locally, but not on GHA

@github-actions github-actions Bot removed the github_actions Pull requests that update Github_actions code label Oct 23, 2023
@tgeens
Copy link
Copy Markdown
Contributor Author

tgeens commented Oct 24, 2023 •
edited
Loading

Before this PR, GHA builds/tests already fail for the following versions:

  • v1.17.17-k3s1
  • v1.18.20-k3s1
  • v1.19.16-k3s1

Note that it works locally with ./gradlew test just fine - and I could not figure out why.

The oldest (minor) version that succeeds with GHA builds is v1.20.15-k3s1

@tgeens tgeens requested a review from eddumelendez October 24, 2023 15:22
@eddumelendez
Copy link
Copy Markdown
Member

eddumelendez commented Oct 24, 2023

Thanks for the information, @tgeens! For me even locally fails with the versions you mentioned on M1. It would have been great to test against the minimal version but given our examples were using rancher/k3s:v1.21.3-k3s1, I'm ok with merging this.

@eddumelendez eddumelendez merged commit 0c52c41 into testcontainers:main Oct 24, 2023
@eddumelendez
Copy link
Copy Markdown
Member

eddumelendez commented Oct 24, 2023

Thanks for your contribution, @tgeens ! This is now merged in main branch and it will be part of the next release.

fokion pushed a commit to fokion/testcontainers-java that referenced this pull request Jan 18, 2025
@tgeens @eddumelendez @fokion
Fix K3S start command (testcontainers#7677)
afe1b00 
Replace `--no-deploy` and use `--disable` instead.

Fixes testcontainers#6770

---------

Co-authored-by: Eddú Meléndez <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@eddumelendez eddumelendez eddumelendez approved these changes

Assignees

No one assigned

Labels

modules/k3s type/bug

Projects

None yet

Milestone

1.19.2

Development

Successfully merging this pull request may close these issues.

[Bug]: k3s > 1.24.x will not start

2 participants

@tgeens @eddumelendez