Skip error uninstalling system package #1315
Conversation
|
test integrations |
|
Created or updated PR in integrations repostiory to test this vesrion. Check elastic/integrations#6619 |
|
| switch pkgManifest.Name { | ||
| case "system": | ||
| logger.Debugf("failed to uninstall package %q: %s", pkgManifest.Name, err.Error()) | ||
| default: | ||
| logger.Warnf("failed to uninstall package %q: %s", pkgManifest.Name, err.Error()) | ||
| } |
There was a problem hiding this comment.
Should we also show a Debug message in case system package fails ? It will always fail. Same for asset runner.
If not this code could be just changed to be something like:
// by default system package is part of an agent policy and it cannot be uninstalled
// https://github.com/elastic/elastic-package/blob/5f65dc29811c57454bc7142aaf73725b6d4dc8e6/internal/stack/_static/kibana.yml.tmpl#L62
if err != nil && pkgManifest.Name != "system" {
logger.Warnf("failed to uninstall package %q: %s", pkgManifest.Name, err.Error())
}
return nil
There was a problem hiding this comment.
Yeah, let's log the error only for other packages.
| // by default system package is part of an agent policy and it cannot be uninstalled | ||
| // https://github.com/elastic/elastic-package/blob/5f65dc29811c57454bc7142aaf73725b6d4dc8e6/internal/stack/_static/kibana.yml.tmpl#L62 | ||
| if err != nil && pkgManifest.Name != "system" { | ||
| logger.Warnf("failed to uninstall package %q: %s", pkgManifest.Name, err.Error()) |
There was a problem hiding this comment.
It may be also good to add a comment here about why we are only logging the error and not returning it.
There was a problem hiding this comment.
Added comment to explain it 6e0af5e
💚 Build Succeeded
History
cc @mrodm |
This PR has two goals:
systempackage is uninstalledCurrently,
systempackage is being used by default as part of another agent policy. Because of that, when system or asset tests finish and try to uninstall the package, if this package issystem, this fails with this error:As part of this PR,
assettests has also been checked and fixed since this test runner also performs the uninstallation of the package.