Skip to content

Fix/go14.4.6#62

Closed
kuisathaverat wants to merge 11 commits into
elastic:masterfrom
kuisathaverat:fix/go14.4.6
Closed

Fix/go14.4.6#62
kuisathaverat wants to merge 11 commits into
elastic:masterfrom
kuisathaverat:fix/go14.4.6

Conversation

@kuisathaverat
Copy link
Copy Markdown
Contributor

@kuisathaverat kuisathaverat commented Jul 20, 2020

  • bump go version to 1.14.6
  • split ARM arch
  • split x86 arch
  • split MIPS arch
  • split ppc arch

see https://www.debian.org/releases/stretch/mips/ch02s01.html.en

related to elastic/beats#19677 elastic/beats#11750

@kuisathaverat kuisathaverat added enhancement New feature or request automation Team:Automation Label for the Observability productivity team labels Jul 20, 2020
@kuisathaverat kuisathaverat requested review from a team and urso July 20, 2020 16:07
Comment thread go1.14/armel/rootfs/compilers.yaml Outdated
armv5:
CC: arm-linux-gnueabi-gcc
CXX: arm-linux-gnueabi-g++
arm64:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think arm64 and armv7 can be removed since these compilers are part of different images.

Comment thread go1.14/armhf/rootfs/compilers.yaml Outdated
armv7:
CC: arm-linux-gnueabihf-gcc
CXX: arm-linux-gnueabihf-g++
armv6:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Only the armv7 compiler should be in this file.

Comment thread go1.14/Makefile Outdated
@@ -1,4 +1,4 @@
IMAGES := base main darwin arm mips ppc s390x
IMAGES := base main amd64 darwin arm armel armhf mips ppc s390x
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Perhaps arm should be renamed to arm64.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

And main has effectively become the i386 image.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We'll have to update anything that uses the images to pull the right arch flavor so I guess if we're changing one (main -> amd64) then it won't be too much to rename arm and main...?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes, we have to change the arch we pass when we launch the images, I start to find where this is in beats but I did not found it yet, Do you known where I have to touch to change it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's at https://github.com/elastic/beats/blob/ded2e9912bb669e896b9c8d013dfabdb759023bd/dev-tools/mage/crossbuild.go#L180-L206. Journalbeat's magefile has a custom ImageSelectorFunc in order to use a different image due to debian7 + systemd issues. It will need updated too.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should probably make the image location configurable in Beats to allow for easier testing of the staged images (e.g. export BEATS_GOLANG_CROSSBUILD_IMAGE=docker.elastic.co/observability-ci/golang-crossbuild)

BeatsCrossBuildImage = EnvOr("BEATS_GOLANG_CROSSBUILD_IMAGE", "docker.elastic.co/beats-dev/golang-crossbuild")

https://github.com/elastic/beats/blob/7580c72445ecc6e97cab6fb7bcffc8063abf105b/dev-tools/mage/settings.go#L46

Comment thread go1.14/amd64/rootfs/compilers.yaml
Comment thread go1.14/amd64/rootfs/compilers.yaml Outdated
@kuisathaverat
Copy link
Copy Markdown
Contributor Author

I have checked supported architectures on CentOS, RedHat, Ubuntu, and Debian, the mips64 and ppc64 architectures are not supported by any of those distributions. On Debian, we have compilers for mips64 and ppc64 but do not have binary packages and it is not easy to prepare a bootstrap filesystem to compile all the packages we need. I think we can rid of mips64 and ppc64, these will be the supported architectures are:

Architecture Debian Designation Subarchitecture Flavor
Intel x86-based i386 default x86 machines default
Xen PV domains only xen
AMD64 & Intel 64 amd64
ARM armel Marvell Kirkwood and Orion marvell
ARM with hardware FPU armhf multiplatform armmp
64bit ARM arm64
32bit MIPS (big-endian) mips MIPS Malta 4kc-malta
Cavium Octeon octeon
64bit MIPS (little-endian) mips64el MIPS Malta 5kc-malta
Cavium Octeon octeon
Loongson 3 loongson-3
32bit MIPS (little-endian) mipsel MIPS Malta 4kc-malta
Cavium Octeon octeon
Loongson 3 loongson-3
Power Systems ppc64el IBM POWER8 or newer machines
64bit IBM S/390 s390x IPL from VM-reader and DASD generic

@andrewkroh @urso @elastic/observablt-robots

@kuisathaverat
Copy link
Copy Markdown
Contributor Author

kuisathaverat commented Jul 22, 2020

I have made a local test on how hard is to move down to Debian 8 (libc6-2.18), for amd64 and i386 is easy for the rest of the architectures we need to compile packages, I am not sure it worth it. Debian 7(libc6-2.13) need to compile a bunch of packages on any architecture. In my opinion, we can keep amd64 and i386 on Debian 8, and the rest should use Debian 9.

@urso
Copy link
Copy Markdown

urso commented Jul 23, 2020

I have made a test on how hard is to move down to Debian 8 (libc6-2.18), for amd64 and i386 is easy for the rest of the architectures we need to compile packages, I am not sure it worth it. Debian 7(libc6-2.13) need to compile a bunch of packages on any architecture. In my opinion, we can keep amd64 and i386 on Debian 8, and the rest should use Debian 9.

Can we reduce the amount of changes we introduce here? For the 7.9 release we only need go to be updated to go1.14.6

@kuisathaverat
Copy link
Copy Markdown
Contributor Author

kuisathaverat commented Jul 23, 2020

@urso In that case, I will open another PR with only the change on the go version nothin else.

the changes on this PR are the ones commented on the first comment nothing else, the images are generated with Debian 9, as they are now.

@kuisathaverat kuisathaverat self-assigned this Jul 27, 2020
@kuisathaverat
Copy link
Copy Markdown
Contributor Author

@andresrc @urso @andrewkroh we have to make a decision here, Do we want to split the Docker images in single architectures or not?

@urso
Copy link
Copy Markdown

urso commented Nov 10, 2020

we have to make a decision here, Do we want to split the Docker images in single architectures or not?

In general I'm +1 on splitting. What would be the disadvantages of doing this?

@kuisathaverat
Copy link
Copy Markdown
Contributor Author

we have to make a decision here, Do we want to split the Docker images in single architectures or not?

In general I'm +1 on splitting. What would be the disadvantages of doing this?

I do not see any, the images are easy to maintain, are smaller, and you use only the one you need.

@andrewkroh
Copy link
Copy Markdown
Member

+1 on splitting the images. Hopefully some of the layers can be shared between all of the images to minimize the amount of downloads/storage (but that's just an optimization and certainly not a requirement).

@fearful-symmetry
Copy link
Copy Markdown
Contributor

What's the status on this? It's blocking #71, which we'd like to resolve.

@kuisathaverat
Copy link
Copy Markdown
Contributor Author

What's the status on this? It's blocking #71, which we'd like to resolve.

This was open in Jul, we have to re-work it over the new Go version, I will finish with #72 and the nest thing to do is the split, give me a few days

@urso
Copy link
Copy Markdown

urso commented Dec 17, 2020

we have to re-work it over the new Go version

Beats will not update to go 1.15 for the 7.11 release btw.

@kuisathaverat
Copy link
Copy Markdown
Contributor Author

@urso @fearful-symmetry Do we need it for 7.11? if so I will update this PR.

@urso
Copy link
Copy Markdown

urso commented Dec 17, 2020

AFAICT there is no immedate need for this.

@fearful-symmetry
Copy link
Copy Markdown
Contributor

Yah, I think we're looking at 7.12 for the additional OS support.

@kuisathaverat
Copy link
Copy Markdown
Contributor Author

superseded by #89

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

Labels

automation enhancement New feature or request Team:Automation Label for the Observability productivity team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants