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.

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

feat(nomad): improved nomad support #798

Merged
merged 26 commits into from
Dec 18, 2024
Merged

Conversation

lukasmetzner
Copy link
Contributor

@lukasmetzner lukasmetzner commented Nov 28, 2024

This PR adds better support for our integration with Nomad. Although not officially supported, we would like to improve the developer experience and integrate automated e2e tests to avoid breaking Nomad integrations with future csi-driver releases.

The Terraform module for creating a Nomad (+Consul) cluster on Hetzner Cloud can be found here.

Steps, that still need to be done:

  • Add e2e tests to the CI pipeline
  • After initial nomad-dev-env release, add renovate comment to nomad/dev/main.tf
  • Update docs/nomad/README.md

@lukasmetzner lukasmetzner self-assigned this Nov 28, 2024
Copy link

codecov bot commented Nov 28, 2024

Codecov Report

Attention: Patch coverage is 0% with 174 lines in your changes missing coverage. Please review.

Project coverage is 32.38%. Comparing base (ffaf6ba) to head (53f20dc).
Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
test/e2e/nomad/utils.go 0.00% 174 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #798      +/-   ##
==========================================
- Coverage   35.34%   32.38%   -2.97%     
==========================================
  Files          20       21       +1     
  Lines        1901     2075     +174     
==========================================
  Hits          672      672              
- Misses       1195     1369     +174     
  Partials       34       34              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@lukasmetzner
Copy link
Contributor Author

With respect to version testing:
The community edition only includes support for the current major version.

Enterprise users will have access to more support for the current major version and the two previous major versions. In addition, there are LTS versions available.

Further information can be found here. Running enterprise products requires the license to be present on startup.

@lukasmetzner lukasmetzner force-pushed the improved-nomad-support branch from 447c284 to 63b87ba Compare December 9, 2024 12:30
@apricote
Copy link
Member

Regarding the tested versions:

I think we should start with the latest version only. If there are many Nomad enterprise users who would like to see the csi-driver tested against older/enterprise versions we can reach out to our contacts at Hashicorp to see if they can provide the licenses for that.

nomad/dev/Makefile Outdated Show resolved Hide resolved
nomad/hcloud-csi-controller.hcl Outdated Show resolved Hide resolved
test/e2e/nomad/e2e_test.go Show resolved Hide resolved
@apricote
Copy link
Member

One more thing: There are a lot of files with missing newline at eof. Could you add it to all the files?

@lukasmetzner lukasmetzner force-pushed the improved-nomad-support branch from d01edf6 to db2f3f6 Compare December 13, 2024 09:27
@lukasmetzner lukasmetzner marked this pull request as ready for review December 13, 2024 10:09
@lukasmetzner lukasmetzner requested a review from a team as a code owner December 13, 2024 10:09
@lukasmetzner lukasmetzner force-pushed the improved-nomad-support branch from 8d65b94 to acc0b71 Compare December 13, 2024 10:20
Copy link
Member

@apricote apricote left a comment

Choose a reason for hiding this comment

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

Very nice! Just a nit :)

nomad/start_nomad.sh Outdated Show resolved Hide resolved
nomad/stop_nomad.sh Outdated Show resolved Hide resolved
docs/nomad/README.md Outdated Show resolved Hide resolved
nomad/skaffold.yaml Outdated Show resolved Hide resolved
nomad/README.md Outdated Show resolved Hide resolved
nomad/README.md Outdated Show resolved Hide resolved
nomad/skaffold.yaml Outdated Show resolved Hide resolved
test/e2e/nomad/e2e_test.go Outdated Show resolved Hide resolved
test/e2e/nomad/utils.go Outdated Show resolved Hide resolved
@lukasmetzner lukasmetzner force-pushed the improved-nomad-support branch from 13f92f4 to c501c5e Compare December 16, 2024 13:47
lukasmetzner and others added 9 commits December 16, 2024 16:12
Co-authored-by: Julian Tölle <[email protected]>
Co-authored-by: Julian Tölle <[email protected]>
Accidentally removed the wrong commit from the history. This reintroduces the changes and adds a controller count of 2 back to the official guide.
Co-authored-by: Jonas L. <[email protected]>
Co-authored-by: Jonas L. <[email protected]>
@lukasmetzner lukasmetzner force-pushed the improved-nomad-support branch from 7b970e4 to 2e2220a Compare December 16, 2024 15:13
@lukasmetzner lukasmetzner merged commit 4dbea7d into main Dec 18, 2024
7 checks passed
@lukasmetzner lukasmetzner deleted the improved-nomad-support branch December 18, 2024 10:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants