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

Make OpenEBS as a helm extension #3651

Merged
merged 5 commits into from
Jan 23, 2024

Conversation

juanluisvaladas
Copy link
Contributor

Description

OpenEBS is migrated from spec.extensions.storage to helm. This is done for consistency with the rest of the documentation and allows to untie the OpenEBS version from the k0s version.

This commit makes three changes:
1- Add the corresponding docs changes
2- Deprecate the storage API in kubebuilder
3- Force a panic if OpenEBS is defined as both a helm extension and a
storage extension

Fixes #3433

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update

How Has This Been Tested?

  • Manual test
  • Auto test added

Checklist:

  • My code follows the style guidelines of this project
  • My commit messages are signed-off
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • I have checked my code and corrected any misspellings

@juanluisvaladas juanluisvaladas requested a review from a team as a code owner October 26, 2023 18:35
@twz123 twz123 added this to the 1.29 milestone Oct 26, 2023
@juanluisvaladas
Copy link
Contributor Author

After giving this some more thought we cannot merge this because those with dynamic config may end up in a crash loop unable to modify their configuration.

How should we deal with the situation when someone has both defined?

Copy link
Contributor

github-actions bot commented Dec 9, 2023

The PR is marked as stale since no activity has been recorded in 30 days

@github-actions github-actions bot added Stale and removed Stale labels Dec 9, 2023
@juanluisvaladas
Copy link
Contributor Author

I had forgotten about this and became stale. Can you please review this? the issue is solved

docs/storage.md Outdated Show resolved Hide resolved
docs/storage.md Outdated Show resolved Hide resolved
docs/storage.md Outdated

### Installing 3rd party storage solutions

Follow your storage driver's installation instructions. Note that the Kubelet installed by k0s uses a slightly different path for its working directory (`/varlib/k0s/kubelet` instead of `/var/lib/kubelet`). Consult the CSI driver's configuration documentation on how to customize this path.
Copy link
Member

Choose a reason for hiding this comment

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

This should better mention the --data-dir flag, since the path is /path/to/data-dir/kubelet, and /var/lib/k0s happens to be the default for data-dir.

docs/storage.md Outdated Show resolved Hide resolved
docs/storage.md Outdated Show resolved Hide resolved
docs/storage.md Show resolved Hide resolved
docs/examples/openebs.md Outdated Show resolved Hide resolved
pkg/component/controller/extensions_controller.go Outdated Show resolved Hide resolved
docs/examples/openebs.md Outdated Show resolved Hide resolved
docs/examples/openebs.md Show resolved Hide resolved
docs/examples/openebs.md Outdated Show resolved Hide resolved
@twz123 twz123 modified the milestones: 1.29, 1.30 Jan 10, 2024
Copy link
Member

@jnummelin jnummelin left a comment

Choose a reason for hiding this comment

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

I think we need to have a migration integration test for this.

docs/examples/openebs.md Outdated Show resolved Hide resolved
docs/examples/openebs.md Show resolved Hide resolved
@juanluisvaladas juanluisvaladas marked this pull request as ready for review January 17, 2024 15:57
inttest/openebs/openebs_test.go Outdated Show resolved Hide resolved
inttest/openebs/openebs_test.go Outdated Show resolved Hide resolved
inttest/openebs/openebs_test.go Outdated Show resolved Hide resolved
@juanluisvaladas
Copy link
Contributor Author

I'm running the tests locally, they have failed so many times at this point I think they could be legit...

@juanluisvaladas
Copy link
Contributor Author

Apparently the failures are legit, I broke something, perhaps the flag --single?

@juanluisvaladas
Copy link
Contributor Author

OK found the issue fixing it now

docs/examples/openebs.md Outdated Show resolved Hide resolved
docs/examples/openebs.md Outdated Show resolved Hide resolved
docs/examples/openebs.md Outdated Show resolved Hide resolved
docs/examples/openebs.md Outdated Show resolved Hide resolved
docs/examples/openebs.md Outdated Show resolved Hide resolved
inttest/openebs/openebs_test.go Outdated Show resolved Hide resolved
pkg/component/controller/extensions_controller.go Outdated Show resolved Hide resolved
pkg/component/controller/extensions_controller.go Outdated Show resolved Hide resolved
docs/examples/openebs.md Outdated Show resolved Hide resolved
docs/storage.md Show resolved Hide resolved
@@ -168,6 +168,7 @@ func GetWorkerFlags() *pflag.FlagSet {
}

var availableComponents = []string{
constant.ApplierManagerComponentName,
Copy link
Member

Choose a reason for hiding this comment

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

Just realizing this is newly added here. Could you update the string in the docs accordingly?

--disable-components strings disable components (valid items: autopilot,control-api,coredns,csr-approver,endpoint-reconciler,helm,konnectivity-server,kube-controller-manager,kube-proxy,kube-scheduler,metrics-server,network-provider,node-role,system-rbac,worker-config)

Signed-off-by: Juan-Luis de Sousa-Valadas Castaño <[email protected]>
juanluisvaladas and others added 4 commits January 22, 2024 16:02
OpenEBS is migrated from `spec.extensions.storage` to helm. This is done
for consistency with the rest of the documentation and allows to untie
the OpenEBS version from the k0s version.

This commit makes three changes:
1- Add the corresponding docs changes
2- Deprecate the storage API in kubebuilder
3- Ignore the extensions configuration if OpenEBS is defined as both a
helm extension and a storage extension

Signed-off-by: Juan-Luis de Sousa-Valadas Castaño <[email protected]>
Co-authored-by: Tom Wieczorek <[email protected]>
This is required for the OpenEBS test, this is extracted to a separate
commit to make history a bit cleaner.

Signed-off-by: Juan-Luis de Sousa-Valadas Castaño <[email protected]>
Signed-off-by: Juan-Luis de Sousa-Valadas Castaño <[email protected]>
Co-authored-by: Tom Wieczorek <[email protected]>
Signed-off-by: Juan-Luis de Sousa-Valadas Castaño <[email protected]>
@juanluisvaladas juanluisvaladas merged commit 7e7fbcb into k0sproject:main Jan 23, 2024
76 checks passed
@twz123 twz123 modified the milestones: 1.30, 1.29 Feb 22, 2024
@twz123 twz123 mentioned this pull request Jun 4, 2024
16 tasks
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.

Question: How can i Update OpenEBS?
3 participants