-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[7.16] [Fleet] Enable auto-upgrades for Stack-aligned packages during Fleet setup #112831
Comments
Pinging @elastic/fleet (Team:Fleet) |
Agreed that apm should be part of the auto-upgrade list, but only be upgraded if previously installed. Regarding the auto-upgrade policies: I think we should keep it simple, with as little configs as necessary to start with. When a user facing config is anyways necessary for some packages, then I think it is also ok to offer that for the apm package. Usually new features come with a default value in the apm-server binary, so if the package policy has not been upgraded, the upgraded apm-server binary would fall back to the internal default value. In summary, I don't see a use case for apm to not upgrade the package policy, but if it keeps things simpler, letting users chose to opt-out is ok. |
Apologies for the delay. This is clearly an improvement, but I'm worried about users being able to disable auto-upgrade. I'd like @dominiqueclarke 's input on this. My worry is that it means that the custom UI for the integration now must support old versions of the integration if that is the case. What are your thoughts @dominiqueclarke is this something we can work with? |
I still don't understand how we can support people running older packages indefinitely. If a user can run old packages (and therefore mappings) indefinitely, in a few years time we could find users with very old mappings that we no longer want to support running 'new' versions of the stack (think an 8.21.0 version of the agent with 8.3.0 era mappings due to an old un-upgraded package). If other products have a strategy for working with this I'd be curious to hear it. None of our competitors support multiple simultaneous versions in the synthetics space or similar products. It's hard to see these multiple versions easing customer pain in a significant way. Beats largely got it right previously, mappings should be upgraded with the stack version. |
@joshdover I have two questions that will help me better think about this issue
|
Thanks all for taking a look.
I do want to clarify that the default behavior that we are currently implementing in #112702 is for package policy upgrades to be off by default. It sounds to me that this is likely not the behavior we'd prefer long-term for the APM and Synthetics packages (but maybe is appropriate for Endpoint). I don't believe any problems have been raised, but I'd like to check that this is acceptable for 7.16.
I agree and I don't see a lot of value in this for end-users either. I think we should opt for an "always upgrade" experience if at all possible, and then handle the exceptions (eg. Endpoint) with dedicated UX that nags users to upgrade (with missing features in the UI, for example).
Yes, the upgrade will always use the most recent version of the package that is compatible with the
Yes, we only upgrade policies to the latest version of the base package that has been installed, which already respects this condition. |
For APM the behavior should be that by default package upgrades also trigger a package policy upgrade. This is aligned with what we had discussed as default behavior for stack-aligned packages. From what I understood automatic package upgrades was only planned for |
For stack-aligned packages my understanding is that it would always upgrade to the package with the exact same stack versioin. Is this not true? |
Yes, for APM, that is accurate. No changes in upgrade behavior for the APM package are planned for 7.16. For other packages that already upgrade during Fleet Setup (system, elastic_agent, fleet_server, endpoint [if installed]), we will not be upgrading the package policies in 7.16 either. In 8.0, we can change the default of this setting, however I am trying to determine if need to be able to set the default of this setting on a package-by-package basis. For example, Endpoint may not want to rollout new agent policies to 200k+ agents on Kibana upgrade.
I thought we had discussed using the Once we implement #112095, this would change to just install the version that is on disk for packages that opt to bundle. |
Right, that's what we discussed last. I think as long as we always specify the package version as the min supported Kibana version we should be fine. |
@joshdover I'm concerned about the policy editing flow. When the stack version ships, Synthetics will ship with a new custom UI that exposes new configuration fields. When users go back to edit their old policies, they'll see these new fields but the spec won't support them, leading to silent errors and user confusion. To mitigate this we'll either need to A.) Introduce additional complexity into the UI to check capabilities from the spec before showing fields or B.) Keep the package policies updated when the package is updated. For this reason and the others stated, I believe it would be best to ship in 7.16 with both package auto upgrades and package policy auto upgrades for Synthetics if possible. |
Thanks for the feedback, @dominiqueclarke.
When is this scheduled to ship? Is this going to affect 7.16 or only 8.x?
Is it possible that upgrading the package policies to the package version that supports the custom UI could fail in any way (eg. due to a field conflict)? If so, it seems to me that we will have to be able to support something like (A) even if we do attempt to upgrade package policies on Kibana upgrades. @jen-huang @kpollich I'm not yet familiar with how these custom integration UIs work in the policy editor, especially in the context of package policy upgrades. Do either of you have opinions on how we should proceed here? |
We are scheduled to ship 3 new capabilities in 7.16 supported by new changes in the custom UI. We have not made any changes to existing fields or mapping in the integration spec since our first version shipped in 7.13.0. We have only made additive changes, adding additional fields, mappings, and data streams. There could be a conflict in an unreleased version prior to |
We discussed this today and decided the following, given some conditions:
Edit: I've opened an issue to describe the scope of changes for 7.16: #114306 |
@joshdover
From our first release, Will this pose an issue? |
We discussed this on Slack, and decided we'll be moving the default value for the One more question for @ogupte and @dominiqueclarke: I’m curious how much effort is involved in allowing the custom UI to handle both newer and older versions of the package? While we have the plumbing in place to support automatically upgrading the package and policy, this is the first release that these features are shipping in and I’m a bit hesitant to opt in to this behavior by default before we have "battle-tested" this functionality more. There are other reasons these UIs will need to support older versions of the packages anyways:
If we agree that handling more than one package version is the best approach for 7.16, I propose that we do not move forward on #114306 and instead treat these issues with these two packages as a bug that can be fixed after feature freeze in the custom package editor UIs. |
@joshdover From my perspective, it would be a medium lift to provide a short term solution for the features shipping in 7.16 until 8.0.0 and a large lift to fully retrofit the UI to support every version. From a product perspective, the Synthetics team is still keen to avoid supporting multi versions. With regards to your three points about why we may need to support older versions, I'm interested in hearing more about the last point regarding elastic agents. I think understanding more about how elastic agents are upgraded along with the stack would be helpful as this sounds like another point of failure to custom UI package authors to consider. cc: @andrewvc |
One thing that is important from my perspective is that we don't build custom code into Fleet to migrate certain packages. If certain packages need a custom migration and have a custom UI, this migration should happen as part of this code. If we temporarily need an Elastic Agents are upgraded last. It might be that older Elastic Agents stay around for a long time. If the data ship is incompatible with the mapping of a more recent package, ingest pipelines need to be used to convert the data. |
@ruflin Under what circumstances would older Elastic Agents stay around for a long time? We may have newer versions of the package that expose additional capabilities in heartbeat, that older Elastic Agents may not support. |
While I completely understand this desire, I don't think this is something we can reasonably get away with given the possibility that a failure may occur (if for no other reason than a bug). That said, I don't think we need complete 100% support of older policies. Let me clarify. We can instead ensure that the your custom policy editor UI can take an older policy as input and always save to the most recent version. This would mean that editing a policy for an integration at 0.1.0 would always save the integration at 0.3.0 (or whatever the version the base package is at). This would at least help us ensure that any package policies that cannot be upgraded automatically in the background can be edited by the user (which would essentially trigger an upgrade too). Note: this may already be how your UI is built, but if not, this would need to be fixed for 7.16. After discussing this in more depth with @kpollich, we're not very happy with our options with a quick implementation for #114306 in 7.16. Chiefly, the error handling logic needs quite a bit more in-depth considerations as well as the accompanying UX for when a package policy upgrade fails. I propose we pare down the changes for 7.16 to the following:
This means that:
As move forward with #111858 in 8.x, we'll be spending more time on error handling logic and UX which will allow us to consider opting these packages in for policy auto-upgrades as well. We should also have more feedback from users on the package policy upgrade feature which should allow us to plug any holes or bugs in the feature before enabling it by default. |
@joshdover thanks for all the thought that's gone into this. Allowing editing a policy to save it as the latest version sounds like a good option, though this could still leave policies which never need to be edited on older versions if not auto upgraded. A few last questions.
|
This piece has been completed in #114641. |
@joshdover @dominiqueclarke and others thanks for putting in the work to refine this. It's really important we simplify the experience and the codepaths here. This brings us into parity with other products and reduces the number of ways in which things can go wrong. To answer the three points @joshdover raised earlier.
We should make this impossible, there is no competing product that lets you downgrade. If it breaks, it breaks. We should work to prevent that.
Same answer as above.
Heartbeart can solve this by supporting older/newer configs within some window (maybe 1 version). Heartbeat has always worked fine with backward/forward compat (it just ignores new settings), so I'm not worried here. |
Yes, this would be the case for the 7.16 release.
We could pass in whatever info is needed into the rendering functions here: Lines 472 to 493 in ba8abc4
I imagine we could provide a
If you think this would work for you, please let us know and we put a quick POC together so you could test against. If not, please let us know what you would need instead.
Yep, I totally agree. I don't feel incredibly comfortable making this change in a release we may be supporting for some time with a brand new upgrade feature on by default, with the risk that it could possibly break the packages (lower severity) or Fleet setup in general (higher severity). We can likely make sure these policy upgrades are isolated enough to not completely break anything critical, but I don't think we have time to deliver a good UX around this. That said, a failure during the policy upgrade is not very likely to affect the Agents in the policy, which is the most critical component. So we either:
Note that in either case, I think we need to solve the editor problem with a solution as I outlined above since failure today is possible. @ruflin do you have any opinion on which risk wins out here?
++ we need to move this to a package manifest field to override or set the default for the package setting we have. cc @mostlyjason |
One thought I have is that, as far as I'm aware, correct me if we're wrong @joshdover, the auto upgrade policies feature is currently off by default, but users can opt into keeping their policies up to date. So if a user opts into keeping policies up to date, they could encounter all of the above problems we've described with auto upgrading, even if we aren't always auto updating synthetics package by default. So these problems will be present regardless of whether or not synthetics policies are auto upgraded by default. We still have UX challenges with failures for users who opt into keeping their policies up to date and have update failures. If we choose not to auto upgrade synthetics policies by default, we will have a subset of users who opt in to having their policies updated, and a subset of users who do not. Now we have to manage both of those subsets. If we force synthetics policy updates, we could focus just the challenges in the auto upgrading code path. If we don't force synthetics policy updates, we have to contend with the challenges in two code paths. Edit: Overall, I do feel like providing a mechanism to upgrade the policy when edited is good solution for 7.16, as that is my primary concern for UI breaks when it comes to not auto upgrading the policies. |
That's largely correct. One key difference is when these package policy upgrades are executed. If we were to enable this by default for these packages, then they will run when Fleet Server starts up (on ESS) instead of when a user clicks the package upgrade button. This creates a slightly unique UX challenge in that there is no user actively using the UI when Fleet Server starts up and triggers the package (and policy) upgrades. My primary concern is around stability of Fleet setup and this is the risk I want to avoid in this release.
This is true. Are you concerned about how we support these two scenarios in support cases or handling this in code? I believe the code problem will be solved by the "edit as upgrade" option we've discussed.
We'll work on getting a POC that you can test against for this idea, hopefully today. |
I am concerned about handling these two problems in code for users who opt into "keep my policies updated" and users who do not. For users who do not opt in, the edit flow proposed will be sufficient for 7.16. For users who do opt in, we will have all the same problems (potential migration errors) that we have discussed for always updating policies for all synthetics users. For synthetics, we will update our package to add defaults to required fields, so hopefully for these users the opt in auto updates will not fail. However, if they do, what is the flow there? Do you believe the edit flow proposed will be sufficient in this case as well, by notifying the user to edit/resave policies that have conflicts? |
Great.
Long-term, no I do not think what we have is adequate. We need to define the error handling UX in general as well as determine the different classes of errors that are possible. This is something that is on our immediate horizon and we'll communicate our plans as soon as we determine them. Auto upgrading policies is definitely a case that we need and plan to support in a future release. |
I created a Proof of Concept in #114914 that allow extensions to opt into targeting the latest version fo a package on the "Edit Package Policy" view via a new flag in the |
@dominiqueclarke @ogupte if you all could review the above POC today, we can solidify our decision on the path forward for 7.16 and close this issue. |
Hearing no objections to our plan, I'm going to close this issue. Summary of the changes: APM and Synthetics base packages will be auto-upgraded (if previously installed) when a user upgrades to 7.16. Their package policies will not be, however once #114914 merges, custom policy editor UIs may opt-in to new behavior that allows them to force users to upgrade packages when editing their policies. This allows the editors to only have to handle 1 schema rather than multiple versions. The final changes on the Fleet side related to this will be handled in #114914 and no other changes for 7.16 are planned. |
During Fleet setup, we currently attempt to upgrade packages for any packages specified in the
kibana.yml
preconfiguration, in addition to any packages specified in the auto-upgrade packages (currently only the Endpoint package).In order to support smooth upgrades for packages that need to be aligned with the Stack we should:
Support upgrading package policies for auto-upgraded packagesWe have decided this is out of scope for 7.16 See discussion starting around #112831 (comment)
The basics will be covered by #106048 which also adds a
Keep policies up-to-date
setting that a user can disable (off by default).Open questions:
In this initial phase, I propose that we:
policy_upgrade_strategy
field to the package manifest with the following possible options:always
- always attempt to keep package policies in sync with package upgradesoptional
- (default) allow the user to configure this settingalways
to override the user-specified settingalways
is specified on a packageIn future phases we could:
skip_on_stack_upgrade
option - if a package is upgraded on a Stack upgrade, do not upgrade the policies at the same time. Otherwise, the behavior would be the same asoptional
.[Done] Add additional packages to the auto-upgrade list
This is necessary for on-prem clusters that may not have packages like APM in their preconfiguration. These packages should only upgraded if they were previously installed by the user (like Endpoint today).
Proposed auto-upgrade behavior by Stack package:
cc @simitt @Mpdreamz could you take a look at this proposal and confirm what is or isn't needed for 7.16 and 8.0 for APM?
cc @andrewvc could you take a look at this proposal and confirm what is or isn't need for 7.16 and 8.0 for Synthetics?
The text was updated successfully, but these errors were encountered: