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

Versions existence should be validated #2455

Closed
mtparet opened this issue Jan 21, 2020 · 8 comments
Closed

Versions existence should be validated #2455

mtparet opened this issue Jan 21, 2020 · 8 comments
Labels
discuss We need to figure this out >enhancement Enhancement of existing functionality

Comments

@mtparet
Copy link

mtparet commented Jan 21, 2020

Proposal

I tried upgrading an existing cluster to 7.5.2 but this version is not already out. As the operator does not support downgrading, I cannot now go back to 7.5.1.. so my cluster is stuck until 7.5.2 is out.

Bug Report

What did you do?

Upgrade to 7.5.2.

What did you expect to see?

Refuse the kubectl apply of an unkown version or allow downgrading in this case

What did you see instead? Under which circumstances?
Error pulling the image

elasticsearch-logs-es-data-2-3           0/1     Init:ErrImagePull   0          74s

Error downgrading to 7.5.1

for: "clusters/k8s-aws-prod-2/bundles/elastic-system/elasticsearch-logs.yaml": admission webhook "elastic-es-validation-v1beta1.k8s.elastic.co" denied the request: Elasticsearch.elasticsearch.k8s.elastic.co "elasticsearch-logs" is invalid: spec.version: Invalid value: "7.5.1": Downgrades are not supported

Environment

  • ECK version:

    1.0

  • Kubernetes information:

    • EKS 1.14
$  kubectl version
Client Version: version.Info{Major:"1", Minor:"16", GitVersion:"v1.16.0", GitCommit:"2bd9643cee5b3b3a5ecbd3af49d09018f0773c77", GitTreeState:"clean", BuildDate:"2019-09-18T14:36:53Z", GoVersion:"go1.12.9", Compiler:"gc", Platform:"linux/amd64"}
Server Version: version.Info{Major:"1", Minor:"14+", GitVersion:"v1.14.9-eks-c0eccc", GitCommit:"c0eccca51d7500bb03b2f163dd8d534ffeb2f7a2", GitTreeState:"clean", BuildDate:"2019-12-22T23:14:11Z", GoVersion:"go1.12.12", Compiler:"gc", Platform:"linux/amd64"}
@anyasabo
Copy link
Contributor

7.5.2 is released so this should be resolved. I'm not sure how we might protect against this in the future, will need to think more on it

@pebrc
Copy link
Collaborator

pebrc commented Feb 6, 2020

We discussed offline, a few thoughts:

  • we don't want to explicitly check against released versions of the Elastic Stack as this would either force us to release new versions of ECK when new versions of the stack are released or do some kind of network request to figure out what the currently valid versions are. The latter being problematic for air-gapped environments
  • what we could do instead is compare the submitted version in the manifest with the currently running versions as documented in the metadata of the pods.
    • we would allow any version that is equal or greater to the max version of the running pods.
    • this would allow users to 'downgrade' successfully if they accidentally configured an non-existent version (because that pod would never be running)
    • this would also allow us to run the version validation in the operator process itself (currently we are only validating in the webhook because we need the old version of the manifest to compare against. This is problematic in it's own right because not every ECK installation has a webhook)

@thbkrkr thbkrkr added >enhancement Enhancement of existing functionality discuss We need to figure this out labels Feb 28, 2020
@sebgl
Copy link
Contributor

sebgl commented Mar 30, 2020

@charith-elastic
Copy link
Contributor

I wonder if we can use the container registry defined in the manifest and issue an API request to grab the list of available tags to check against. The downside is that it still requires a network call per submission (unless some clever caching is used) and might not be 100% reliable due to network issues and restrictive registry configurations.

@anyasabo
Copy link
Contributor

anyasabo commented Mar 30, 2020

It also happened to me the other day because the beats 7.6.2 release notes are already there in the current branch, so I made the bad assumption I just missed a patch release and tried to use it.

For Charith's suggestion:
I think we can. It looks like there's a decent amount of libs already (e.g. the GCR one here). I don't even think we need to get that fancy, we can get the list of current tags for the default repo on startup, and only make a new call over the network if it's something we don't know about already. It's not 100% because tags can be removed (so we might have a now-invalid tag whitelisted in the cache), but it should still be a big improvement over the current situation.

It's also difficult because we would want to parse the image pull secret, and as Charith said it's possible that pods don't have access to the registry but kubelets do. I'd think we could fail open in those cases though, and only reject a request if we can successfully list the tags and the requested tag is not present.

I am def not an image registry connoisseur though, so it is possible I am missing some big edge cases. And either way it seems like it would be simpler to implement the "no downgrades if there are any running pods at a higher version" logic.

@haipengzzz
Copy link

Has this problem been solved?

@pebrc
Copy link
Collaborator

pebrc commented Mar 21, 2022

While the problem of validating the version submitted by users has not been solved we have since introduced an escape hatch to go back to the last known good version in case a mistake has been made. This is documented here: https://www.elastic.co/guide/en/cloud-on-k8s/master/k8s-common-problems.html#k8s-common-problems-version-downgrade

I am therefore closing this issue for now.

@pebrc pebrc closed this as completed Mar 21, 2022
@mtparet
Copy link
Author

mtparet commented Mar 21, 2022

Thanks, I was also thinking closing this ticket. Happy to see there is a solution now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss We need to figure this out >enhancement Enhancement of existing functionality
Projects
None yet
Development

No branches or pull requests

7 participants