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 to Ubuntu 20.04 #1635

Merged
merged 14 commits into from
Oct 16, 2020
Merged

Update to Ubuntu 20.04 #1635

merged 14 commits into from
Oct 16, 2020

Conversation

Arri98
Copy link
Contributor

@Arri98 Arri98 commented Sep 17, 2020

Description

Updated installer and dockerfile to Ubuntu 20.04 and gcc7:

  • Updated dockerfile for Ubuntu 20.04.
  • installUbuntuDeps checks Ubuntu version and installs gcc7 for Ubunutu 18 and 20 and gcc5 for other versions for retrocompability.
  • Updated python2 to python3.
  • Cpplint installed with conan and removed cpplint file which used python2.
  • Audited npm packages and updated those who don't need aditional changes.
  • Increased sleep time after rabitmq in initDockerLicode to prevent docker image for failing at start

With these changes docker images can be compiled from Ubuntu 16.04, 18.04 and 20.04 and installation from source works for Ubuntu 16.04 and 20.04 (Installation in Ubuntu 18.04 has not been tested but should work). With gcc7 additional warnings appear in some libraries that are no longer been updated but installation works.

[] It needs and includes Unit Tests

Changes in Client or Server public APIs

No changes where made

[] It includes documentation for these changes in /doc.

@Arri98
Copy link
Contributor Author

Arri98 commented Sep 17, 2020

Build is failing in erizo lint and I think it is because python 3 is installed instead of python 2 and the cpplint file that erizo has needs python 2.

From CircleCI:
Scanning dependencies of target lint
Executing lint
/bin/sh: 1: /opt/licode/erizo/src/../utils/cpplint.py: Permission denied
make[3]: *** [CMakeFiles/lint.dir/build.make:58: CMakeFiles/lint] Error 126
make[2]: *** [CMakeFiles/Makefile2:196: CMakeFiles/lint.dir/all] Error 2
make[1]: *** [CMakeFiles/Makefile2:203: CMakeFiles/lint.dir/rule] Error 2
make: *** [Makefile:155: lint] Error 2

Exited with code exit status 2

Copy link
Contributor

@jcague jcague left a comment

Choose a reason for hiding this comment

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

Great contribution! I have some minor comments to understand some of the updates better. Nothing makes me think this won't work, but I first want to test it in different environments, because it implies many indirect changes, mainly new versions of third-party dependencies. Also, old hidden bugs could arise due to updating GCC, so I prefer to be a bit careful.

@@ -1,9 +1,12 @@
FROM ubuntu:16.04
FROM ubuntu:20.04
Copy link
Contributor

Choose a reason for hiding this comment

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

👏


MAINTAINER Lynckia

WORKDIR /opt

#Configure tzdata
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#Configure tzdata
# Configure tzdata


MAINTAINER Lynckia

WORKDIR /opt

#Configure tzdata
ENV TZ=Europe/Madrid
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need to set the timezone here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

With Ubuntu 20, when running apt-get update the library tzdata will install and stop the docker build to ask the user for their timezone. The problem is that even when the user introduces the timezone, docker build gets stuck so I had to set the timezone before.

@@ -62,7 +64,7 @@ check_result() {
run_rabbitmq() {
echo "Starting Rabbitmq"
rabbitmq-server -detached
sleep 3
sleep 5
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@@ -91,6 +100,12 @@ install_conan(){
pip3 install conan==1.21
}

install_cpplint(){
sudo pip3 install cpplint==1.5.4
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we also need to add cpplint as a dep into the installMacDeps.sh, but I can do it in a different PR if you don't have access to a Mac.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes we should because all scripts no longer call the cpplint.py file that was removed. I don't have access to a Mac so if you could do it it would be perfect.

@@ -21,6 +21,7 @@ elif [ "$(expr substr $(uname -s) 1 5)" == "Linux" ]; then
fi

export ERIZO_HOME=$ROOT/erizo
export PATH=$HOME/.local/bin:$PATH; conan
Copy link
Contributor

Choose a reason for hiding this comment

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

is this something needed now with Ubuntu 20.04?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure if it is because Ubuntu 20.04 but without it the script wouldn't find conan. For what I have seen, pip doesn't install conan globally and the shell wouldn't find it if I don't specify the path.

Dockerfile Outdated
@@ -29,6 +32,8 @@ RUN ./installErizo.sh -dfeacs && \
./../nuve/installNuve.sh && \
./installBasicExample.sh

RUN ldconfig /opt/licode/build/libdeps/build/lib
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need to use ldconfig?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When running erizo tests the script wasn't able to find the libraries installed in /opt/licode/build/libdeps/build/lib

1: Test command: /opt/licode/erizo/build/debug/test/tests
1: Test timeout computed to be: 10000000
1: /opt/licode/erizo/build/debug/test/tests: error while loading shared libraries: libavresample.so.2: cannot open shared object file: No such file or directory
1/1 Test #1: all ..............................***Failed 0.00 sec

I had to run ldconfig to add the links to the path. I didn't want to modify the dockerfile and run ldfconfig in the installUbuntuDeps file but it wasn't working as intended.

"version": "4.17.15",
"resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.15.tgz",
"integrity": "sha512-8xOcRHvCjnocdS5cpwXQXVzmmh5e5+saE2QGoeQmbKmRS6J3VQppPOIt0MnmE+4xlZoumy0GPG0D0MVIQbNA1A=="
"version": "4.17.20",
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there ar reason to update the version of these libraries with Ubuntu 20.04? If not, I'd rather submit this package-lock.json files in another different PR to simplify the way we can test stuff.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is no particular reason, but there were some medium and high severity vulnerabilities. Most of them could be fixed automatically with npm audit fix but installation works without updating the libraries.

@@ -21,6 +21,7 @@ cd $ROOT/nuve
sleep 5

export ERIZO_HOME=$ROOT/erizo/
export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:$ROOT/build/libdeps/build/lib
Copy link
Contributor

Choose a reason for hiding this comment

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

I think I'm a bit lost with the new usage of LD_LIBRARY_PATH and ldconfig in this PR. Can you explain in a few words why do we need them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As I commented before, when updating to Ubuntu 20 the script wheren't able to find the libdeps libraries. This was the first attempt to add the path to the libraries to the bash and fixed some of the problems but not all of them. I have to check if this command is redundant after running ldconfig.

sudo update-alternatives --install /usr/bin/gcc gcc /usr/bin/gcc-5 60 --slave /usr/bin/g++ g++ /usr/bin/g++-5
if [[ $(lsb_release -rs) == "18.04" ]] || [[ $(lsb_release -rs) == "20.04" ]] ; then
echo "Installing gcc7"
sudo apt-get install -qq git make gcc-7 g++-7 python3-pip libssl-dev cmake pkg-config liblog4cxx-dev rabbitmq-server mongodb curl autoconf libtool automake -y
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can split this apt-get install command and install mongodb, rabbitmq, curl, etc outside of the condition. Another alternative is to set the GCC version on a global var and use it with something like sudo apt-get install ... gcc-$(gcc_version)....

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I'll change it.

@Arri98
Copy link
Contributor Author

Arri98 commented Oct 5, 2020

Changes in the last commit:

  • Reverted package update.
  • Removed an unnecessary LD_LIBRARY_PATH export.
  • Installed conan with sudo so I don't have to export the route.
  • Small refractor to installUbuntuDeps.

Copy link
Contributor

@jcague jcague left a comment

Choose a reason for hiding this comment

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

This looks good to me! We have been testing it for a while in a dev environment with 0 issues.

@jcague
Copy link
Contributor

jcague commented Oct 16, 2020

@Arri98 thanks a lot! this is a great contribution!

@jcague jcague merged commit 8a1eb05 into lynckia:master Oct 16, 2020
@aalonsog
Copy link
Member

👏

Arri98 added a commit to Arri98/licode that referenced this pull request Apr 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants