-
Notifications
You must be signed in to change notification settings - Fork 717
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
E2E TestEnterpriseTrialLicense fails #3163
Comments
So I managed to run the test locally (after spending some time figuring out which pubkey/privkey/license I need to extract), and... it succeeded! :| |
From the operator logs:
It looks like the license controller did its job of removing the cluster license secret. err := c.Get(
types.NamespacedName{
Namespace: esCluster.Namespace,
Name: esv1.LicenseSecretName(esCluster.Name),
},
&license,
)
if err != nil {
if apierrors.IsNotFound(err) {
if isTrial(current) {
// this is the case if the user has started a trial on the ES cluster level
// e.g. from Kibana when trying to access a commercial feature.
// While this is not a supported use case we tolerate it to avoid a bad user
// experience because trials can only be started once on the ES cluster level
return nil
}
// no license linked to this cluster. Revert to basic.
return startBasic(ctx, updater)
}
return err
} But it didn't happen so I guess it may have taken the
I think the following race condition could have happened:
This seems to be confirmed by the logs, I see a bunch of elasticsearch-controller logs (pasted above), then the observer doing its job but no reconciliation anymore for the elasticsearch-controller. |
I see 2 ways to solve this:
I think I prefer option 2. |
We used to rely on the license fetched by the observers on a regular basis, but this can lead to a race condition as described in elastic#3163 (comment). Instead, let's keep things simple, and do a synchronous request to get the license as part of the normal reconciliation, to then decide whether or not it should be updated. In case of error (eg. Elasticsearch req timeout), enqueue a new reconciliation, but don't abort the current reconciliation.
) Fixes #3163. Fixes a bug introduced in #3150. We used to rely on the license fetched by the observers on a regular basis, but this can lead to a race condition as described in #3163 (comment). Instead, let's keep things simple, and do a synchronous request to get the license as part of the normal reconciliation, to then decide whether or not it should be updated. In case of error (eg. Elasticsearch req timeout), enqueue a new reconciliation, but don't abort the current reconciliation. This PR also adds some minor refactoring of the code, and avoids a call to start_basic if the current license is already basic.
https://devops-ci.elastic.co/job/cloud-on-k8s-e2e-tests-stack-versions/101/testReport/
The text was updated successfully, but these errors were encountered: