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

Support for generating docker image on kibana build #28380

Merged
merged 33 commits into from Feb 5, 2019

Conversation

Projects
None yet
6 participants
@mistic
Copy link
Member

mistic commented Jan 9, 2019

This PR adds the ability to build the Docker image through the current kibana build process.
Almost every feature currently present on https://github.com/elastic/kibana-docker/ was ported to this implementation. However, in this first version, we'll have the follow assumptions:

  • The docker image will be build from local kibana artifacts generated from the kibana build itself with. The kibana docker image will be generated with the default yarn build however, if we just wanna build the docker image we can just do yarn build --docker and skip the other packages.
  • The defined supported elasticsearch version will always match exactly the current kibana version from where we are build (with version qualifier and snapshot/release tag info too )

I believe this is enough for a first version and our current needs. One thing to improve in the future is the way we are generating the kibana_vars on /src/dev/build/tasks/os_packages/docker_generator/resources/bin/kibana-docker

I believe once we finish this feature #26903 we can replace this manual list just from getting all the configs we have for the kibana cli from this config service.

Closes #22934 , elastic/kibana-docker#115

mistic added some commits Jan 7, 2019

fix(NA): build cli to mention docker package in a help cmd output. fi…
…x(NA): generated config for kibana_yml tempalte.

@mistic mistic self-assigned this Jan 9, 2019

@mistic mistic requested a review from tylersmalley Jan 9, 2019

@elasticmachine

This comment has been minimized.

Copy link

elasticmachine commented Jan 9, 2019

@mistic mistic requested a review from joshdover Jan 9, 2019

@elasticmachine

This comment was marked as outdated.

Copy link

elasticmachine commented Jan 9, 2019

@joshdover
Copy link
Member

joshdover left a comment

One thing I noticed is this build target doesn't produce a .sha1.txt file. Is that intentional?

Show resolved Hide resolved ...uild/tasks/os_packages/docker_generator/templates/dockerfile.template.js Outdated
Show resolved Hide resolved src/dev/build/cli.js Outdated
docs(NA): update description on --docker build flag option
Co-Authored-By: mistic <tiagoffcc@hotmail.com>
@elasticmachine

This comment was marked as outdated.

Copy link

elasticmachine commented Jan 17, 2019

mistic added some commits Jan 17, 2019

Merge branch 'support-for-generating-docker-image-on-build' of github…
….com:mistic/kibana into support-for-generating-docker-image-on-build
@elasticmachine

This comment was marked as outdated.

Copy link

elasticmachine commented Jan 17, 2019

mistic added some commits Jan 17, 2019

@elasticmachine

This comment was marked as outdated.

Copy link

elasticmachine commented Jan 17, 2019

mistic added some commits Jan 30, 2019

@mistic

This comment has been minimized.

Copy link
Member Author

mistic commented Jan 30, 2019

@tylersmalley I think I just fixed the problem. Can you try again please?

@elasticmachine

This comment was marked as outdated.

Copy link

elasticmachine commented Jan 30, 2019

@tylersmalley

This comment has been minimized.

Copy link
Member

tylersmalley commented Jan 31, 2019

@jarpy would you mind taking a look at this PR to bring Docker over to the Kibana build?

mistic added some commits Feb 1, 2019

@elasticmachine

This comment was marked as outdated.

Copy link

elasticmachine commented Feb 2, 2019

@tylersmalley

This comment has been minimized.

Copy link
Member

tylersmalley commented Feb 3, 2019

retest

@elasticmachine

This comment was marked as outdated.

Copy link

elasticmachine commented Feb 3, 2019

#
# --elasticsearch.startupTimeout=60

kibana_vars=(

This comment has been minimized.

@tylersmalley

tylersmalley Feb 4, 2019

Member

@jarpy is there a reason we can't take the same approach as ES is with matching environment variables, except for ours would match uppercase?

https://github.com/elastic/elasticsearch/blob/master/distribution/docker/src/docker/bin/docker-entrypoint.sh#L51-L62

This comment has been minimized.

@tylersmalley

tylersmalley Feb 4, 2019

Member

In the meantime, @mistic would you mind updating this list as it's recently been updated in the kibana-docker repo: elastic/kibana-docker#128

This comment has been minimized.

@mistic

This comment has been minimized.

@jarpy

jarpy Feb 4, 2019

Contributor

You could do that, but:

You'll have to do something for flags that don't have dots, like regionmap.

You'll have to ask the users to switch to mixed-case env vars. Unlike ES, Kibana has mixed-case flags, so it essentially encodes information in character case. You can't smash case to upper or lower without losing that information. That's why this horrible "translation table" exists. Arguably, we should have just asked for mixed-case, 1:1 mapped env vars to begin with. That might be a good change for 7.x.

This comment has been minimized.

@jarpy

jarpy Feb 4, 2019

Contributor

Note that there was a time when we couldn't use dots in env vars, but that issue is now resolved. It does, however, explain why we also s/_/./

@tylersmalley

This comment has been minimized.

Copy link
Member

tylersmalley commented Feb 4, 2019

Still looks like this is generating an archive that is substantially larger than what we had previously

master:
oss: 167M
default: 264M

support-for-generating-docker-image-on-build:
oss: 486M (+191%)
default: 757M (+187%)

@elasticmachine

This comment was marked as outdated.

Copy link

elasticmachine commented Feb 4, 2019

mistic added some commits Feb 4, 2019

@mistic

This comment has been minimized.

Copy link
Member Author

mistic commented Feb 4, 2019

@tylersmalley I just ask some help from @jarpy, in order to understand how we are doing the things for the RM, and I think the problem is solved!

@jarpy

jarpy approved these changes Feb 4, 2019

Copy link
Contributor

jarpy left a comment

Great stuff! Thank you.

@elasticmachine

This comment was marked as outdated.

Copy link

elasticmachine commented Feb 4, 2019

@elasticmachine

This comment has been minimized.

Copy link

elasticmachine commented Feb 4, 2019

@tylersmalley
Copy link
Member

tylersmalley left a comment

LGTM all checks out now

@tylersmalley tylersmalley merged commit 3a1d4ad into elastic:master Feb 5, 2019

2 checks passed

CLA Commit author is a member of Elasticsearch
Details
kibana-ci Build finished.
Details

tylersmalley added a commit to tylersmalley/kibana that referenced this pull request Feb 5, 2019

mistic added a commit that referenced this pull request Feb 5, 2019

@mistic

This comment has been minimized.

Copy link
Member Author

mistic commented Feb 5, 2019

6.x: f56cf4f

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment