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

Update base Ubuntu image to 20.04 #26905

Merged
merged 5 commits into from
May 31, 2023
Merged

Update base Ubuntu image to 20.04 #26905

merged 5 commits into from
May 31, 2023

Conversation

jakule
Copy link
Contributor

@jakule jakule commented May 25, 2023

Contributes to #26856

@gzdunek gzdunek mentioned this pull request May 26, 2023
# teleport built on any newer Ubuntu version will not run on Centos 7 because
# of this.
# This Dockerfile makes the "build box": the container used to
# build Teleport Connect.
Copy link
Member

Choose a reason for hiding this comment

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

This makes it sound like this image is used only for that, but I don't think that it's true. build.assets/Makefile includes quite a few targets that refer to $(BUILDBOX) but are not related to Connect.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This makes it sound like this image is used only for that

That was according to my best knowledge yesterday. Today, I checked a few places and update the comment 😅

# This Dockerfile makes the "build box": the container used to build official
# releases of Teleport and its documentation.
#
# Check the README to learn how to safely introduce changes to Dockerfiles.
Copy link
Member

Choose a reason for hiding this comment

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

Do you think that line was unnecessary? 😶‍🌫️

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This image is not used for our "official releases" or "documentation". I reverted the "README" line, but TBH I modified this file dozen of times and I wasn't aware that we have README here 🙈

Copy link
Contributor

Choose a reason for hiding this comment

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

What you think about adding a line of explanation why we don't use the latest ubuntu? (because the native dependency depends on glibc)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I meant a comment about using Ubuntu 20.04 (which builds Connect), not Centos.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, fixed 😅

@jakule jakule changed the title Update base Ubuntu image to 22.04 Update base Ubuntu image to 20.04 May 26, 2023
Copy link
Contributor

@gzdunek gzdunek left a comment

Choose a reason for hiding this comment

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

I tested this build https://drone.platform.teleport.sh/gravitational/teleport/24549/5/1 (20.04) on Ubuntu 20.04 and it works.

# This Dockerfile makes the "build box": the container used to build official
# releases of Teleport and its documentation.
#
# Check the README to learn how to safely introduce changes to Dockerfiles.
Copy link
Contributor

Choose a reason for hiding this comment

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

What you think about adding a line of explanation why we don't use the latest ubuntu? (because the native dependency depends on glibc)

@jakule jakule marked this pull request as ready for review May 31, 2023 13:50
@github-actions github-actions bot requested review from flyinghermit and zmb3 May 31, 2023 13:51
## LIBFIDO2 ###################################################################

# Build libfido2 separately for isolation, speed and flexibility.
FROM buildpack-deps:18.04 AS libfido2
FROM buildpack-deps:20.04 AS libfido2
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we bump to 22.04, since it's the newer LTS?

Copy link
Contributor

Choose a reason for hiding this comment

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

If we build Connect on 22.04 then it will require glib from 22.04, so it won’t work on 20.04.
A native module that we rebuild causes a dependency on glib.

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we built connect on centos7, then?

There's also the possibility of bootstrapping glibc ourselves - it takes some work, but frees us from having to use old distros to build. I think @jakule and myself talked about this before.

(In any case this comment ain't a blocker.)

Copy link
Contributor Author

@jakule jakule May 31, 2023

Choose a reason for hiding this comment

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

IMO we should avoid building more stuff on CentOS 7. Node.js doesn't provide builds for CentOS 7 (or even Ubuntu 18.04 at this point). If we want to build everything on CentOS 7 we would need to build Node.js from the source. Node seems to build fine on CenOS 7, but the community doesn't provide official support for such an old version of Glibc. They are also not testing it on some very old platforms, which case cause some more problems in the future.
The main question at this point is how many people are even using Connect on Ubuntu older than 20.04 (and other distros with older glibc). I'd assume that CentOS 7 is not very popular for Linux desktop distributions, and we should be fine with Ubuntu 20.04+.

The one thing that I'd advocate for is to split the main buildbox and Connect container. By doing so, we will be able to use ubuntu:latest in our CI and keep whatever OS we want for Connect build. This should also speed up the Connect build itself, as Connect doesn't need 90% of the Buildbox dependencies (Go, Rust, linters, etc).

Copy link
Contributor

Choose a reason for hiding this comment

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

Also keep in mind that the current version of Electron embeds Node v18 that requires glibc >= 2.28 (so Connect won't even start on Centos7, even if we build it on it).
Ubuntu 20.04 uses glibc 2.31. Theoretically, we are losing potential users with OSes that use glibc 2.28 - 2.30, but I don't think it is a big deal?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

CentOS 8 Stream and Debian 10 are using 2.28 (random website that seems to have correct data https://repology.org/project/glibc/versions). Both are EOL the next year. Again, it's hard to estimate the market usage, but it looks like these users will be affected. If this is an issue we could split the image (as I mentioned above) and switch the base image to Debian 10 (should be simple as Ubuntu is based on Debian).
CC @zmb3

Copy link
Contributor

Choose a reason for hiding this comment

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

The one thing that I'd advocate for is to split the main buildbox and Connect container

+1

@jakule jakule enabled auto-merge May 31, 2023 15:12
@jakule jakule added this pull request to the merge queue May 31, 2023
Merged via the queue into master with commit 95b4339 May 31, 2023
@jakule jakule deleted the jakule/update-docker-ubuntu branch May 31, 2023 15:47
@public-teleport-github-review-bot

@jakule See the table below for backport results.

Branch Result
branch/v11 Failed
branch/v12 Failed
branch/v13 Create PR

jakule added a commit that referenced this pull request Jun 1, 2023
As described here golang/go#40334 incorrect system configuration can lead user.LookupGroup to return ENOENT. We've already added a workaround #18981, but this one place seems to be forgotten. I also added ESRCH to ignored errors as newer glibc returns it.

The error was exposed after updating our docker image to Ubuntu 20.04 #26905 which includes newer Glibc.
jakule added a commit that referenced this pull request Jun 1, 2023
As described here golang/go#40334 incorrect system configuration can lead user.LookupGroup to return ENOENT. We've already added a workaround #18981, but this one place seems to be forgotten. I also added ESRCH to ignored errors as newer glibc returns it.

The error was exposed after updating our docker image to Ubuntu 20.04 #26905 which includes newer Glibc.
github-actions bot pushed a commit that referenced this pull request Jun 1, 2023
As described here golang/go#40334 incorrect system configuration can lead user.LookupGroup to return ENOENT. We've already added a workaround #18981, but this one place seems to be forgotten. I also added ESRCH to ignored errors as newer glibc returns it.

The error was exposed after updating our docker image to Ubuntu 20.04 #26905 which includes newer Glibc.
github-actions bot pushed a commit that referenced this pull request Jun 1, 2023
As described here golang/go#40334 incorrect system configuration can lead user.LookupGroup to return ENOENT. We've already added a workaround #18981, but this one place seems to be forgotten. I also added ESRCH to ignored errors as newer glibc returns it.

The error was exposed after updating our docker image to Ubuntu 20.04 #26905 which includes newer Glibc.
github-actions bot pushed a commit that referenced this pull request Jun 1, 2023
As described here golang/go#40334 incorrect system configuration can lead user.LookupGroup to return ENOENT. We've already added a workaround #18981, but this one place seems to be forgotten. I also added ESRCH to ignored errors as newer glibc returns it.

The error was exposed after updating our docker image to Ubuntu 20.04 #26905 which includes newer Glibc.
jakule added a commit that referenced this pull request Jun 2, 2023
As described here golang/go#40334 incorrect system configuration can lead user.LookupGroup to return ENOENT. We've already added a workaround #18981, but this one place seems to be forgotten. I also added ESRCH to ignored errors as newer glibc returns it.

The error was exposed after updating our docker image to Ubuntu 20.04 #26905 which includes newer Glibc.
jakule added a commit that referenced this pull request Jun 2, 2023
As described here golang/go#40334 incorrect system configuration can lead user.LookupGroup to return ENOENT. We've already added a workaround #18981, but this one place seems to be forgotten. I also added ESRCH to ignored errors as newer glibc returns it.

The error was exposed after updating our docker image to Ubuntu 20.04 #26905 which includes newer Glibc.
jakule added a commit that referenced this pull request Jun 2, 2023
As described here golang/go#40334 incorrect system configuration can lead user.LookupGroup to return ENOENT. We've already added a workaround #18981, but this one place seems to be forgotten. I also added ESRCH to ignored errors as newer glibc returns it.

The error was exposed after updating our docker image to Ubuntu 20.04 #26905 which includes newer Glibc.
jakule added a commit that referenced this pull request Jun 6, 2023
* Update base Ubuntu image to 22.04

* Revert the ubuntu image to 20.04

* Update the Dockerfile comment

* Add CentOS 7 note

* Add Connect note
jakule added a commit that referenced this pull request Jun 6, 2023
* Update base Ubuntu image to 22.04

* Revert the ubuntu image to 20.04

* Update the Dockerfile comment

* Add CentOS 7 note

* Add Connect note
jakule added a commit that referenced this pull request Jun 7, 2023
* Update base Ubuntu image to 22.04

* Revert the ubuntu image to 20.04

* Update the Dockerfile comment

* Add CentOS 7 note

* Add Connect note
jakule added a commit that referenced this pull request Jun 8, 2023
* Update base Ubuntu image to 20.04 (#26905)

* Update base Ubuntu image to 22.04

* Revert the ubuntu image to 20.04

* Update the Dockerfile comment

* Add CentOS 7 note

* Add Connect note

* Move Connect build to a new Docker container (#27175)

* Move Connect build to a new Docker container

* Update comments

* Update comments
Remove unused packages and unused arguments

* Always use UID=1000 for building teleterm.
github-merge-queue bot pushed a commit that referenced this pull request Jun 22, 2023
* Update base Ubuntu image to 20.04 (#26905)

* Update base Ubuntu image to 22.04

* Revert the ubuntu image to 20.04

* Update the Dockerfile comment

* Add CentOS 7 note

* Add Connect note

* Move Connect build to a new Docker container (#27175)

* Move Connect build to a new Docker container

* Update comments

* Update comments
Remove unused packages and unused arguments

* Always use UID=1000 for building teleterm.
github-merge-queue bot pushed a commit that referenced this pull request Jun 22, 2023
* Update base Ubuntu image to 20.04 (#26905)

* Update base Ubuntu image to 22.04

* Revert the ubuntu image to 20.04

* Update the Dockerfile comment

* Add CentOS 7 note

* Add Connect note

* Move Connect build to a new Docker container (#27175)

* Move Connect build to a new Docker container

* Update comments

* Update comments
Remove unused packages and unused arguments

* Always use UID=1000 for building teleterm.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants