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

Autostart trial #2160

Merged
merged 21 commits into from
Nov 29, 2019
Merged

Autostart trial #2160

merged 21 commits into from
Nov 29, 2019

Conversation

thbkrkr
Copy link
Contributor

@thbkrkr thbkrkr commented Nov 22, 2019

This commit fixes two bugs:

  • Always create the trial status secret in the operator namespace 53014de
  • Store the license secret name/namespace in the trial status secret ccb03d8 ; in order to reconcile the license secret when the trial status changes

Adds the support for the enterprise trial license:

  • Support trial license in the license controller b15fad9
  • Support trial license in the elasticsearch controller 1d29473
  • Use an annotation to remember that trial has been started d1b7cb9
  • E2E test for the trial license 5726783

Brings a small optimization:

  • Reconcile es cluster license only when es is reachable 64a2f91

Resolves #1003.


Quick recap about how trial license works. 3 controllers are involved:

trial-controller

Watches:

  • Secrets labeled {"license.k8s.elastic.co/type": "enterprise-trial"}
  • Secrets named trial-status

Reconciles:

  • create trial-status secret if it does not exist (store the public key for the trial-enterprise license)
  • updates the user license secret with the license signature (note: the license type label of the secret is updated in enterprise)

license-controller

Watches:

  • Elasticsearch clusters
  • Secrets labeled {"license.k8s.elastic.co/type": "enterprise"}

Reconciles:

  • upserts a secret in the namespace of the Elasticsearch cluster containing the signature of its license

elasticsearch-controller

Watches:

  • Secrets owned by ES clusters:

Reconciles:

  • Update the license (ES API call)

@thbkrkr thbkrkr added >enhancement Enhancement of existing functionality v1.0.0 labels Nov 22, 2019
Copy link
Contributor

@barkbay barkbay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really familiar with this part of the code and I still want to do some tests but overall lgtm!

I left a comment about state consistency (not a new topic actually)

I was also wondering how to start a new trial if a user has upgraded its clusters to a new major version, which would require a call to /_license/trial_status I guess.

pkg/controller/elasticsearch/license/apply.go Outdated Show resolved Hide resolved
@thbkrkr
Copy link
Contributor Author

thbkrkr commented Nov 25, 2019

Looking at the code again, I wonder if I should:

  • remove the trial status that is not used since we don't apply the enterprise-trial license stored in the secret like another license (we just call the API to start the trial)
  • rename the license type enterprise-trial in trial to be consistent with the license type used by Elasticsearch
  • not use the annotation and prefer rely on the observer to store that the trial has been started (very mixed feelings on this because it does not cost much, and we remain independent from the observer to be able to remove it)
    ?

@pebrc
Copy link
Collaborator

pebrc commented Nov 25, 2019

remove the trial status

unless I am misunderstanding your proposal trial status is used to on the orchestration/ECK level to provide some assurance that trials cannot be restarted etc. If you mean the cluster level trial license secret then yes that could make sense

rename the license type enterprise-trial

The ECK level trial is corresponding to the ECK level enterprise license thus enterprise-trial. On the cluster level I would argue you don't need to create a synthetic license secret at all, an empty license secret with an additional annotation to identify would suffice. That annotation/label can indeed use trial instead of enterprise-trial

not use the annotation and prefer rely on the observer to store

How about doing none of this and doing a synchronous GET + (optional) POST instead?

Copy link
Contributor

@barkbay barkbay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

pkg/controller/elasticsearch/license/apply.go Outdated Show resolved Hide resolved
pkg/controller/elasticsearch/license/apply.go Show resolved Hide resolved
Copy link
Collaborator

@pebrc pebrc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think almost ready to merge!

pkg/controller/common/license/labels.go Outdated Show resolved Hide resolved
pkg/controller/elasticsearch/client/client.go Outdated Show resolved Hide resolved
test/e2e/es/license_test.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@pebrc pebrc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚢

pkg/controller/elasticsearch/client/client.go Outdated Show resolved Hide resolved
@thbkrkr
Copy link
Contributor Author

thbkrkr commented Nov 28, 2019

The last build failed because the LOCAL_SSD_TOTAL_GB quota was reached.

@thbkrkr thbkrkr merged commit c841e15 into elastic:master Nov 29, 2019
@thbkrkr thbkrkr deleted the autostart-trial branch December 16, 2019 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement Enhancement of existing functionality v1.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Start Elasticsearch trial if ECK is running in trial mode
3 participants