Better error handling for flavor detection #6804
Conversation
…gated (elastic#6734)" This reverts commit 09c9abc.
michalpristas
added
bug
…tic-agent into fix/flavors-backwards
|
Pinging @elastic/elastic-agent-control-plane (Team:Elastic-Agent-Control-Plane) |
| // no flavor loaded | ||
| if flavor.Name == "" { | ||
| return acceptAllFn, nil | ||
| } |
There was a problem hiding this comment.
How does this actually happen? Is there a flavor in the registry with an empty name? Intuitively, it makes more sense to me to try to prevent this occurrence in the first place instead of handling it downstream like this.
There was a problem hiding this comment.
actually this path specifically does not happen now. we always return error when this is default. i'm leaving it here as safety net. i can remove it as well if you prefer but i rather have it here
There was a problem hiding this comment.
Maybe add a comment if you want to keep it? Right now, it suggests this can actually happen.
michalpristas
changed the title
| @@ -55,11 +55,15 @@ func Flavor(detectedFlavor string, registryPath string, flavorsRegistry map[stri | |||
| if flavorsRegistry == nil { | |||
| f, err := os.Open(registryPath) | |||
| if err != nil { | |||
| return FlavorDefinition{}, err | |||
| if os.IsNotExist(err) { | |||
There was a problem hiding this comment.
Is there a case where the registryPath is not an empty string?
The manifest is already parsed at the call locations of install.Flavor, what is the usecase where we need to open the manifest again here?
There was a problem hiding this comment.
previously it was used by other code path where registry was not opened before. yes this can be cleaned up now
There was a problem hiding this comment.
but as this does not address the bugs and alters a behavior i will address it separately to avoid introducing side effects
|
This PR adds better error handling for flavor detection when manifest file is not found during extracting tar/zip packages.
Backward upgrade tests now working, un-skipping them
Related issues
Closes #6731
Closes #6729
Closes #6732