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

Use TAR instead of DOCKER build type before 6.7.0 #40723

Merged

Conversation

DaveCTurner
Copy link
Contributor

In 6.7.0 (#39378) we added a build type of DOCKER for the docker images, but
unfortunately earlier versions do not understand this and will reject any
transport messages that mention this build type.

This commit fixes this by reporting TAR instead of DOCKER when talking to older
nodes.

Relates (but does not fix) #40511
Relates #39378

In 6.7.0 (elastic#39378) we added a build type of DOCKER for the docker images, but
unfortunately earlier versions do not understand this and will reject any
transport messages that mention this build type.

This commit fixes this by reporting TAR instead of DOCKER when talking to older
nodes.

Relates (but does not fix) elastic#40511
Relates elastic#39378
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

Copy link
Contributor

@ywelsch ywelsch left a comment

Choose a reason for hiding this comment

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

LGTM

@DaveCTurner
Copy link
Contributor Author

Relates #37617 and #38053 - this issue slipped through the cracks because we do not currently run any rolling-upgrade or mixed-cluster tests using nodes running in Docker.

@jasontedor
Copy link
Member

Thanks for finding the issue so quickly and addressing it @DaveCTurner. This is a really unfortunate issue, where changing an enumeration is not what we traditionally think of as a serialization-breaking change. Sadly, we did not have testing infrastructure in place here either, that could at least help us catch issues like this before shipping.

Copy link
Member

@jasontedor jasontedor left a comment

Choose a reason for hiding this comment

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

LGTM.

@jasontedor jasontedor added blocker :Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts and removed :Delivery/Build Build or test infrastructure labels Apr 2, 2019
@DaveCTurner DaveCTurner added v6.7.1 and removed v6.7.2 labels Apr 2, 2019
@alpar-t
Copy link
Contributor

alpar-t commented Apr 2, 2019

@rjernst do you think this is something the upgrade we do in the packaging tests would have caught? It doesn't seem to be the case, so non of the linked issues would have helped catch this. If this is indeed the case I think we should create a new one with what we think the solution to this problem is. To me it seems that randomizing the distribution type in rolling upgrade and full packaging tests would be the way to go ( either on a per test or on a per node basis ). This is something we can could consider for TestClusters.

@jasontedor
Copy link
Member

@atorok The upgrade tests we do in the packaging tests would not have caught this. Those are only a single-node full cluster restart. This issue arises in a mixed cluster only.

@DaveCTurner DaveCTurner merged commit 2ff0bdd into elastic:master Apr 2, 2019
@DaveCTurner DaveCTurner deleted the 2019-04-02-docker-build-type-bwc branch April 2, 2019 12:18
DaveCTurner added a commit that referenced this pull request Apr 2, 2019
In 6.7.0 (#39378) we added a build type of DOCKER for the docker images, but
unfortunately earlier versions do not understand this and will reject any
transport messages that mention this build type.

This commit fixes this by reporting TAR instead of DOCKER when talking to older
nodes.

Relates (but does not fix) #40511
Relates #39378
DaveCTurner added a commit that referenced this pull request Apr 2, 2019
In 6.7.0 (#39378) we added a build type of DOCKER for the docker images, but
unfortunately earlier versions do not understand this and will reject any
transport messages that mention this build type.

This commit fixes this by reporting TAR instead of DOCKER when talking to older
nodes.

Relates (but does not fix) #40511
Relates #39378
@alpar-t
Copy link
Contributor

alpar-t commented Apr 2, 2019

thanks @jasontedor that's what I taught. I added #40732

DaveCTurner added a commit that referenced this pull request Apr 2, 2019
In 6.7.0 (#39378) we added a build type of DOCKER for the docker images, but
unfortunately earlier versions do not understand this and will reject any
transport messages that mention this build type.

This commit fixes this by reporting TAR instead of DOCKER when talking to older
nodes.

Relates (but does not fix) #40511
Relates #39378
DaveCTurner added a commit that referenced this pull request Apr 2, 2019
In 6.7.0 (#39378) we added a build type of DOCKER for the docker images, but
unfortunately earlier versions do not understand this and will reject any
transport messages that mention this build type.

This commit fixes this by reporting TAR instead of DOCKER when talking to older
nodes.

Relates (but does not fix) #40511
Relates #39378

This commit is the changes that were supposed to be included in the preceding
one, abe509a, but weren't because of a missing
`git add`.
gurkankaymak pushed a commit to gurkankaymak/elasticsearch that referenced this pull request May 27, 2019
In 6.7.0 (elastic#39378) we added a build type of DOCKER for the docker images, but
unfortunately earlier versions do not understand this and will reject any
transport messages that mention this build type.

This commit fixes this by reporting TAR instead of DOCKER when talking to older
nodes.

Relates (but does not fix) elastic#40511
Relates elastic#39378
@mark-vieira mark-vieira added the Team:Delivery Meta label for Delivery team label Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocker >bug :Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts Team:Delivery Meta label for Delivery team v6.7.1 v7.0.0-rc2 v7.2.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants