Conversation
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
Trends 🧪 |
cmd/install.go
Outdated
| Long: installLongDescription, | ||
| RunE: installCommandAction, | ||
| } | ||
| cmd.Flags().BoolP(cobraext.UninstallPackageFlagName, "u", false, cobraext.UninstallPackageFlagDescription) |
There was a problem hiding this comment.
It feels a little odd (to me) that we use elastic-package install -u to uninstall a package. I think it would be clearer to create an elastic-package uninstall command instead. Even in terms of implementation this would be cleaner IMO — for example, it doesn't really make sense to check any compatibility conditions when uninstalling a package.
There was a problem hiding this comment.
I agree with uninstall.
Regarding the compatibility conditions... I need this feature somewhere in elastic-package, so that I can implement the support matrix between packages and stacks (elastic/integrations#537 (comment)). I planned to use the tool to check if the CI should spawn the stack, because the package should be compatible.
If you have other ideas for the support check or a better place to put it, feel free to share them.
There was a problem hiding this comment.
No no, I think the compatibility check the way you have it implemented for elastic-package install is just fine. I was saying it is not needed for elastic-package uninstall, that's all.
There was a problem hiding this comment.
This might help clarify my suggestion: I would remove the -u option from this PR and make it just about installing a package. Then put up a separate PR that implements elastic-package uninstall (and in that PR I think we don't need the compatibility checks).
There was a problem hiding this comment.
I removed uninstallation from this PR.
|
|
||
| // CheckConditions method compares the given values with conditions in manifest. | ||
| // The method is useful to check in advance if the package is compatible with particular stack version. | ||
| func CheckConditions(manifest PackageManifest, keyValuePairs []string) error { |
There was a problem hiding this comment.
I really like how generic this function is. I have been thinking of adding more conditions as part of elastic/package-registry#670 and I think something like what you have here will come in very handy!
Fixes: #87
Fixes: #8
This PR exposes CLI action to install the current package (work dir), available in the Package Registry.
It introduces also the "check package conditions" feature to verify if the package meets Elastic stack conditions. It will be used by elastic/integrations#537 (comment) to check in advance whether the stack can expose the package.
How to test the PR:
test/packages/kubernetes.elastic-package buildelastic-package stack up -d(to include the package)elastic-package install(to install the package)Check constraints (Kibana is not required to be up):