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

[Fleet] Add option to toggle search of prerelease packages #122973

Closed
1 task done
Tracked by #225
jsoriano opened this issue Jan 13, 2022 · 53 comments · Fixed by #143853
Closed
1 task done
Tracked by #225

[Fleet] Add option to toggle search of prerelease packages #122973

jsoriano opened this issue Jan 13, 2022 · 53 comments · Fixed by #143853
Assignees
Labels
8.6 candidate QA:Validated Issue has been validated by QA Team:Fleet Team label for Observability Data Collection Fleet team technical debt Improvement of the software architecture and operational architecture

Comments

@jsoriano
Copy link
Member

jsoriano commented Jan 13, 2022

In order to simplify the ways package developers can specify the stability of a package, we are going to rely completely on SemVer. This means that any package version over 1.0.0 and without a prerelease label (like 1.0.0-rc1) will be considered stable, 0.x.x versions or versions with prerelease labels, will be considered prereleases.

This means that we are going to deprecate the release labels in package manifests, and eventually, the different package registry stages (production, staging, snapshot...). More about this in elastic/package-spec#225.

As part of this we are proposing the following changes for Fleet:

  • Add a setting to enable or disable the search of pre-release packages (if possible available both in UI and in configuration).
  • If the setting is enabled, Fleet should search packages adding the prerelease=true parameter (to requests to /search and /categories endpoints).
  • Stop using the experimental=true parameter on requests to the registry.
  • Replace current beta/experimental labels with the following mappings:
    • 0.x versions -> "Technical Preview"
    • x.x.x-beta... -> "Beta"
    • x.x.x-rc... -> "Release Candidate"
    • x.x.x-preview... -> "Technical Preview"
    • Other prerelease labels won't be allowed in the spec, but if they appear, default to "Beta".

prerelease parameter is being added to the registry in elastic/package-registry#785. To keep compatibility with current Kibana versions, experimental=true keeps current behaviour, including prerelease packages in search requests.

Demo

@jsoriano jsoriano added the Team:Fleet Team label for Observability Data Collection Fleet team label Jan 13, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@jsoriano
Copy link
Member Author

jsoriano commented Feb 1, 2022

Updated description after internal discussion about how to map prerelease tags to UI labels as replacement of current "Beta"/"Experimental" ones.

The recommendation would be to use the following convention to replace current labels with the following mappings:

  • 0.x versions -> "Technical Preview"
  • x.x.x-beta... -> "Beta"
  • x.x.x-rc... -> "Release Candidate"
  • x.x.x-preview... -> "Technical Preview"
  • Other prerelease labels won't be allowed in the spec (as of Deprecate the release tag package-spec#256), but if they appear, default to "Beta".

cc @akshay-saraswat @mostlyjason.

@mostlyjason
Copy link
Contributor

Add a setting to enable or disable the search of pre-release packages (if possible available both in UI and in configuration).

@jsoriano I don't see a UI feature today that offers this ability. Only a small subset of integrations are experimental so have you see it be a significant issue for users?

Replace current beta/experimental labels with the following mappings

This part sounds good. @jen-huang I think we'd need to coordinate Fleet UI with the EPR changes?

@jsoriano
Copy link
Member Author

jsoriano commented Feb 7, 2022

Add a setting to enable or disable the search of pre-release packages (if possible available both in UI and in configuration).

@jsoriano I don't see a UI feature today that offers this ability. Only a small subset of integrations are experimental so have you see it be a significant issue for users?

Correct, there is no feature today to disable the search of experimental packages, and they are always searched. We could continue like this by now, only moving from experimental to prerelease. If we add a configuration flag, we can keep it enabled by default by now if we want to keep this behaviour.

But I think that this will be more needed in the future.

Currently one reason to have few in-development packages in the public registry is that we have multiple registries (snapshot/staging), so not everything is promoted to the production one. We have plans to get rid of these promotion approach as part of the migration to the v2 storage, in that scenario the only way for developers to publish in-development packages will be using prerelease versions, and without a toggle, users won't be able to opt-out from these packages.

Another reason is that we want to expand the packages ecosystem to more developers and teams, possibly also out of Elastic, and we want to offer them a simple way to publish in-development versions of packages, instead of the ~three ways we have now.

Summarizing, I would suggest to implement the toggle, so this is ready when needed or requested by users. If we don't want it to be so used by now, we can keep it enabled by default, and only available through configuration files. We could postpone the decision of making it visible in the UI and/or disabling it by default.

@jen-huang jen-huang added the technical debt Improvement of the software architecture and operational architecture label Feb 7, 2022
@jsoriano
Copy link
Member Author

All public instances of the registry support the new prerelease parameter now (experimental is supported too for backwards compatibility).

@mostlyjason
Copy link
Contributor

Sounds good! I'd recommend keeping the same behavior in the UI by default for now, which means showing experimental and higher. I agree it'd be nice to offer users a UI feature to filter on version, but we should define/implement that when it becomes a priority for UX. We have several other UI features we are focusing on currently that are higher up. I'll leave it up the engineering team what is the MVP way to enable prerelease packages for development users.

we want to offer them a simple way to publish in-development versions of packages

@jsoriano Do you plan to distinguish in-development versions of packages intended only for internal team's use from "Technical Preview" packages that are intended for end-users?

@jsoriano
Copy link
Member Author

@jsoriano Do you plan to distinguish in-development versions of packages intended only for internal team's use from "Technical Preview" packages that are intended for end-users?

We want to fully rely on semantic versioning for publicly available packages. If a development team wants to publish a package as technical preview, available to users, they must use a prerelease suffix (for example 1.2.0-preview1). Also, all 0.x versions are considered technical previews.
Regarding the toggle, if we implement it, such packages would be available only to users with it enabled.

I agree with you that we would need to think this well from the UX perspective. Maybe another option, instead of a toggle, would be to always show all packages, but require an additional confirmation to install or upgrade to a prerelease version.

@mostlyjason
Copy link
Contributor

mostlyjason commented Feb 15, 2022

In that case it sounds like we need to show all packages by default. I think that is already the case today. We need to continue showing experimental packages because some users may have already installed those, and it'd be unexpected if they disappeared. We have a label on each card on the browse page, and a label right next to the add button so hopefully users are aware that its a technical preview. Before adding confirmation steps, perhaps we wait to see if we get any feedback that users are confused by this?

image

@jsoriano
Copy link
Member Author

@mostlyjason yes, SGTM to show all packages by default in searches, this will help discovering non-GA features.

I still think that we will need at least something to prevent users of GA packages to upgrade to unintentionally upgrade to a non-GA version. If we consider the labels are enough, then no need to add any setting or confirmation dialog, we can wait to see if users find this necessary.

Then as part of this task we would only need to add the mappings for the new labels, and the use of the prerelease parameter instead of the deprecated experimental.

@mostlyjason
Copy link
Contributor

I still think that we will need at least something to prevent users of GA packages to upgrade to unintentionally upgrade to a non-GA version

Are you saying a package could upgrade from GA to non-GA? I thought the version for a given package could only increase, not decrease? Wouldn't that be a breaking change to remove a GA feature? Or are you saying that a user who uses GA releases normally, would unintentionally add a non-GA release of a different package?

@jsoriano
Copy link
Member Author

I still think that we will need at least something to prevent users of GA packages to upgrade to unintentionally upgrade to a non-GA version

Are you saying a package could upgrade from GA to non-GA?

It is the same as with any other Elastic product, for example Elasticsearch is GA, but before releasing 8.0, there were prerelease versions like 8.0.0-beta1 or 8.0.0-rc2. Maintainers of the Apache package could decide to release a 1.4.0-rc1 prerelease version, this version would be offered to users of the GA Apache 1.3.2 if we don't implement any safeguard.

I thought the version for a given package could only increase, not decrease?

This is true, but there can be prereleases of a greater version (1.4.0-rc1 > 1.3.2).

And this doesn't change with the removal of the release tag, with the release tag it was possible to release 1.4.0 with release: beta after having released 1.3.2 with release: ga.

Wouldn't that be a breaking change to remove a GA feature?

We wouldn't be removing the GA feature, the same way as when we released Elasticsearch 8.0.0-beta1 we were not removing previous Elasticsearch. GA versions continue being available.

Or are you saying that a user who uses GA releases normally, would unintentionally add a non-GA release of a different package?

This is a different case, this would be the case of someone using only GA features, that decides to install a non-GA package, maybe to try something that hasn't been released yet as GA, as could be the case of the synthetics package.

@juliaElastic
Copy link
Contributor

@jsoriano @mostlyjason @joshdover
As part of the release tag change, Integrations UI has to be changed to show package badges based on semver instead of release tag, right? I'm not seeing this feature mentioned in this issue.
Currently the UI shows no badge/beta/experimental coming from the package's release tag field.
This has to change to derive from semver I think and to use the new labeling (ga/technical preview/beta).

@jsoriano
Copy link
Member Author

@juliaElastic yes, this would be the part mentioned in the description as "Replace current beta/experimental labels with the following mappings: ...". Or are you referring to a different thing?

@juliaElastic
Copy link
Contributor

juliaElastic commented Feb 23, 2022

yes, this would be the part mentioned in the description as "Replace current beta/experimental labels with the following mappings: ...". Or are you referring to a different thing?

I see, I guess I wasn't sure if that refers the Integration UI labels, thanks for confirming.
Is the expectation for Fleet to do the mapping from semver to GA/Technical Preview/Beta for the purpose of using as UI badges?

@jsoriano
Copy link
Member Author

Is the expectation for Fleet to do the mapping from semver to GA/Technical Preview/Beta for the purpose of using as UI badges?

Yes, I think this should replace current mappings based on the release tag.

The only other place where we have identified the needing of this mapping has been in elastic-package status, implementation can be found here.

It'd be nice if we could reuse this logic between components, but I don't expect this to change frequently.

@juliaElastic
Copy link
Contributor

Note to Fleet UI team: the previous release tag values are duplicated in a few places, it would be good to move to an enum and rename: https://github.com/elastic/kibana/search?q=%22%27beta%27+%7C+%27experimental%27+%7C+%27ga%27%22

@juliaElastic
Copy link
Contributor

@joshdover
I noticed that on Integrations UI, some packages have empty release and version properties. These are the ones that are split from a package e.g. ActiveMQ Logs and Metrics. Is this a bug? E.g. if base package is beta, the integration card should have beta badge too, right?
image

@joshdover
Copy link
Contributor

These are the ones that are split from a package e.g. ActiveMQ Logs and Metrics. Is this a bug? E.g. if base package is beta, the integration card should have beta badge too, right?

These are not packages, they're actually Beats tutorials that link off to a different app in Kibana. They're only visible when the corresponding package is not yet GA. You can grep around in the codebase for getReplacementCustomIntegrations to find how this replacement happens on the client side.

@juliaElastic
Copy link
Contributor

juliaElastic commented Feb 24, 2022

Yes, I think this should replace current mappings based on the release tag.

The only other place where we have identified the needing of this mapping has been in elastic-package status, implementation can be found here.

It'd be nice if we could reuse this logic between components, but I don't expect this to change frequently.

@jsoriano
we found another place where the release badges (beta/experimental) appear: https://docs.elastic.co/en/integrations

would it make sense to store the mapping logic centrally in registry? I'm not sure if kibana/fleet is the right place to store it, especially if it might be duplicated in other places.

@jsoriano
Copy link
Member Author

@juliaElastic yes, if we start finding more places where this is needed we may need a central place to maintain this convention, I have created elastic/package-spec#286 to discuss this.

I am not sure though if the registry is the best place, because there are situations where the registry won't be available, for example with bundled packages, or with packages installed without the registry (#70582). Also with elastic-package, when showing information about local, not published packages.

Other thing that makes me wonder if this logic should be centralized, is that in principle this is needed purely for UI reasons. Functionally (and regarding support) we should do the same with any non-GA package. With these mappings we are only providing additional information about what to expect, it is like a translation from a technical code to a human-friendly text. And this information can be different in different tools. For example in elastic-package we could decide to show the prerelease tag directly, without any mapping and this would be mostly ok for the target audience. While in Kibana we are also providing additional descriptions, and translations for them. If we bring this logic to the registry, we assume that this can be centrally changed there, but then, would the registry also serve the descriptions? and how would the translations be implemented?

I think we can go by now with specific implementations, and reevaluate later if we find that this is giving problems. We can in any case somehow fix the conventions in the spec as a result of elastic/package-spec#286, so there is a central point where this convention can be referenced as base for the different implementations.

@mostlyjason
Copy link
Contributor

Maintainers of the Apache package could decide to release a 1.4.0-rc1 prerelease version, this version would be offered to users of the GA Apache 1.3.2 if we don't implement any safeguard.

This is an interesting problem because we need to define a ranking algorithm for what we show in Integrations UI. It sounds like the existing algorithm is to show the highest package version? However, it should be to filter on the highest release type first (GA > Beta > Technical Preview), then filter on the highest version. So GA Apache 1.3.2 will outrank 1.4.0-rc1. Does that sound right?

There is a separate problem of how users can install prerelease versions after a GA release has been made. I'm not sure this needs to be part of the MVP release, but is a feature we could add later. I imagine in the API is a workaround that allows developers to install any version they want?

@juliaElastic
Copy link
Contributor

Maintainers of the Apache package could decide to release a 1.4.0-rc1 prerelease version, this version would be offered to users of the GA Apache 1.3.2 if we don't implement any safeguard.

This is an interesting problem because we need to define a ranking algorithm for what we show in Integrations UI. It sounds like the existing algorithm is to show the highest package version? However, it should be to filter on the highest release type first (GA > Beta > Technical Preview), then filter on the highest version. So GA Apache 1.3.2 will outrank 1.4.0-rc1. Does that sound right?

There is a separate problem of how users can install prerelease versions after a GA release has been made. I'm not sure this needs to be part of the MVP release, but is a feature we could add later. I imagine in the API is a workaround that allows developers to install any version they want?

This could be done with a UI toggle or config flag to show only GA packages / show all. The toggle value could be used when checking for latest package versions as well, e.g. not showing Upgrade button when there are no new GA versions available, only prerelease.

@kpollich
Copy link
Member

@elastic/uptime - Could you advise on the above? We are making changes to how prerelease packages interact with Kibana/Fleet and some input would be helpful.

@juliaElastic - It's probably not going to be pretty, but we may have to hardcode a list of exceptions for this behavior since we're shipping a prerelease version of synthetics to all clusters by default right now.

@juliaElastic juliaElastic mentioned this issue Oct 24, 2022
6 tasks
@andrewvc
Copy link
Contributor

@kpollich our package is currently in beta. It seems like we would need to make it v 1.0.0 to appear by default. However, that's kinda at odds with the beta status. Is that the correct understanding?

As long as it's still clearly labeled beta I guess I'm fine with 1.0.0, but I'd like to check with @drewpost and @paulb-elastic specifically on that.

CC @dominiqueclarke as well

@kpollich
Copy link
Member

our package is currently in beta. It seems like we would need to make it v 1.0.0 to appear by default. However, that's kinda at odds with the beta status. Is that the correct understanding?

Yes I'd say this is correct. cc @jsoriano for advice as well, but I think pursuing 1.0.0 makes the most sense. If we're shipping synthetics by default in production environments, it should not on a prerelease version in my opinion.

@juliaElastic
Copy link
Contributor

For now I added an exception for synthetics to be visible even when prerelease packages are hidden (toggle is false).

I think the registry should already work by semver and derive the release label from it (meaning 1.0.0 is going to be GA).
We are moving on the UI as well to this calculation, to ignore the release label.

@andrewvc
Copy link
Contributor

@kpollich our timeline for 1.0 is circa 8.7. We can neither yank support for it now nor artificially move it forward.

I'm fine calling the number 1.0.0 as long as it's still clearly labeled beta.

@juliaElastic
Copy link
Contributor

juliaElastic commented Oct 27, 2022

prerelease parameter is being added to the registry in elastic/package-registry#785. To keep compatibility with current Kibana versions, experimental=true keeps current behaviour, including prerelease packages in search requests.

@jsoriano I discovered a difference in experimental and prerelease flags in the v2 dev epr repo (used by kibana CI). Is this expected?

https://epr-v2.ea-web.elastic.dev/search?package=endpoint&experimental=true
// -> returns endpoint.8.5.0
https://epr-v2.ea-web.elastic.dev/search?package=endpoint&prerelease=true
// -> returns endpoint.8.6.0-dev.0

@andrewvc
Copy link
Contributor

Apologies, I'd somehow missed @juliaElastic 's comment. The exception is perfect for us.

@mrodm
Copy link
Contributor

mrodm commented Oct 28, 2022

@jsoriano I discovered a difference in experimental and prerelease flags in the v2 dev epr repo (used by kibana CI). Is this expected?

https://epr-v2.ea-web.elastic.dev/search?package=endpoint&experimental=true
// -> returns endpoint.8.5.0
https://epr-v2.ea-web.elastic.dev/search?package=endpoint&prerelease=true
// -> returns endpoint.8.6.0-dev.0

@juliaElastic I would say this is the expected behavior. This was the implementation requested and it was implemented here. When the experimental flag is true, as there is a GA version of the package, other non-GA version are discarded.

In this case, for that search, there exists at least one GA version for endpoint, 8.5.0, so other prerelease versions are discarded, like 8.6.0-dev. With the prerelease flag, that behavior is also the expected, prerelease=true returns the prerelease versions.

@juliaElastic
Copy link
Contributor

@mrodm thanks for the context.
I am wondering when is the experimental flag expected to be removed from EPR API?
We have a similar flag in Fleet packages API, which we are making deprecated. Should we keep the behavior in Fleet API that experimental flag is passed as-is to EPR API?

@mrodm
Copy link
Contributor

mrodm commented Oct 28, 2022

I am wondering when is the experimental flag expected to be removed from EPR API?
We have a similar flag in Fleet packages API, which we are making deprecated. Should we keep the behavior in Fleet API that experimental flag is passed as-is to EPR API?

I guess we would need to keep that experimental parameter in EPR for a long time, at least while there is any old Kibana version using it.

But, new Kibana versions (IIRC starting in 8.6) should not be using that parameter in requests against EPR.

@juliaElastic
Copy link
Contributor

But, new Kibana versions (IIRC starting in 8.6) should not be using that parameter in requests against EPR.

Originally I wanted to replace usages of experimental flag to prerelease in Fleet, but now as it turns out they are not behaving the same way. Should we instead just remove experimental=true from the queries to EPR in Fleet?

The prerelease flag will be added to the queries when the setting (UI switch) is turned on.

@mrodm
Copy link
Contributor

mrodm commented Oct 28, 2022

Should we instead just remove experimental=true from the queries to EPR in Fleet?

Yes, I think this is the expected change. experimental=true should be just used by older kibana versions (< 8.6.0)

The prerelease flag will be added to the queries when the setting (UI switch) is turned on.

👍

@juliaElastic
Copy link
Contributor

@dborodyansky @kpollich I am wondering if we should add a confirmation modal or tooltip to the prerelease toggle, because enabling it has some implications that the user might not be aware of. Intuitively it looks like only a UI filter only, but enabling it impacts auto upgrades. Maybe this toggle could be moved to a more prominent settings page in the future.

@kpollich
Copy link
Member

I am wondering if we should add a confirmation modal or tooltip to the prerelease toggle, because enabling it has some implications that the user might not be aware of

Yeah it probably makes sense to display some kind of info dialog about this. A confirmation modal might be a bit heavy, but maybe a tooltip would suffice? Something like this as a tooltip could work:

"Enabling this setting will enable the prerelease channel for all integrations. This will impact automatic integration updates and automatic policy upgrades."

Although upon thinking about this more, it seems a bit heavy-handed to perform auto updates for packages and auto upgrades for policies based on this setting. A user could very easily have a few hundred agents running on the latest stable release of system, enable this setting, then incur an immediate upgrade of all of their agents without realizing it.

Would it be possible to ignore this setting in the context of automatic updates and policy upgrades? I apologize if that creates a lot of rework or churn here, but it feels like the safest option. Happy to pull this into a follow-up issue that can be shipped during FF as well.

@dborodyansky
Copy link
Contributor

The designed (prerelease toggle) UI element in question is intended as display toggle only, and would not be expected to have consequences beyond visibility. Coupling it with a persistent upgrade configuration setting would be problematic. If a configuration setting is required, we should redesign the UI to relocate it out of the browsing experience to where it would be logical for users. Please let me know if I misunderstood something or need to elaborate.

@juliaElastic
Copy link
Contributor

juliaElastic commented Nov 1, 2022

Would it be possible to ignore this setting in the context of automatic updates and policy upgrades? I apologize if that creates a lot of rework or churn here, but it feels like the safest option. Happy to pull this into a follow-up issue that can be shipped during FF as well.

@kpollich I agree that we should avoid any unexpected upgrading to prerelease versions.
I'll remove the code that checks the setting for auto and manual upgrades.
A few things to clarify:

  • When a GA package is installed (e.g. endpoint-8.5.0), and the prerelease toggle is on, do we still want to display the option to manually upgrade to a beta version? Otherwise users wouldn't see the beta versions, only if they uninstall the package first.
  • We have a lot of tests that do "install latest version" on a mock package in beta version. For these we have to use the prerelease=true flag. This starts to become tricky to decide whether to use the prerelease flag without relying on the settings. I will keep testing to make sure there are no accidental auto-upgrades happening.

@kpollich
Copy link
Member

kpollich commented Nov 1, 2022

When a GA package is installed (e.g. endpoint-8.5.0), and the prerelease toggle is on, do we still want to display the option to manually upgrade to a beta version? Otherwise users wouldn't see the beta versions, only if they uninstall the package first.

Yes, we should let users manually upgrade to prerelease/beta packages when the toggle is enabled.

We have a lot of tests that do "install latest version" on a mock package in beta version. For these we have to use the prerelease=true flag. This starts to become tricky to decide whether to use the prerelease flag without relying on the settings. I will keep testing to make sure there are no accidental auto-upgrades happening.

I think for now it makes the most sense to pass the prerelease parameter to fix the tests, but in the future it would probably be good to go through and update the test packages to align better with the new versioning constraints. I'll file an issue for this.

@amolnater-qasource
Copy link

Hi Team,

We have executed 10 testcases for this feature under Fleet test plan at link: Fleet Team 8.6 New features test plan:

Status:
PASS: 04
SKIP: 06

We have skipped 06 cases as the GA version is not available for any technical preview integrations.

  • Hence related testcases are skipped.

Please let us know if anything else from our end.

Thanks

@amolnater-qasource
Copy link

Hi @juliaElastic

While testing on 8.7.0 BC5 we had a new beta Endpoint version available 8.7.2-next.
So, we are successfully able to test this feature on kibana cloud environment.

As discussed over slack, we have added info under the testcases for the self-managed setup available under #143853

Screenshots:
12
13

Further we had some observations while testing this feature:

  • On installing stable endpoint-v8.7.1, the dropdown to select 8.7.2-next version gets removed and upgrade option is only available Settings tab.
  • Further, on upgrading the installed 8.7.2-next the Switch to GA version button is removed from Integration Overview page.

Could you please confirm if this is expected?

Screen Recording:

Browse.integrations.-.Integrations.-.Elastic.-.Google.Chrome.2023-03-15.10-19-48.mp4

Please let us know if anything else is required from our end.
Thanks!!

@juliaElastic
Copy link
Contributor

On installing stable endpoint-v8.7.1, the dropdown to select 8.7.2-next version gets removed and upgrade option is only available Settings tab.
Further, on upgrading the installed 8.7.2-next the Switch to GA version button is removed from Integration Overview page.

Yes, this is expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.6 candidate QA:Validated Issue has been validated by QA Team:Fleet Team label for Observability Data Collection Fleet team technical debt Improvement of the software architecture and operational architecture
Projects
None yet
Development

Successfully merging a pull request may close this issue.