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

Dockerfile lint #689

Merged
merged 14 commits into from
Jun 20, 2023
Merged

Dockerfile lint #689

merged 14 commits into from
Jun 20, 2023

Conversation

Tansito
Copy link
Member

@Tansito Tansito commented Jun 19, 2023

Summary

With the purpose to verify our security processes I started to implement checks in our infrastructure implementation. This PR implements the hadolint linter in all our Dockerfiles.

Details and comments

  • Create a GHA to lint in every PR our Dockerfiles
  • Fix lint errors in our images

ref #690

* DL3008 pin apt-get packages
* DL3013 and SC2261 pin pip versions
* DL3059 consecutively RUN commands
* DL3042 no cache on pip install
* DL3003 use of WORKDIR instead of cd
* DL3045 use WORKDIR using COPY
* DL3009 delete apt-get lists
* DL3015 avoid recommended packages
* DL3045 set WORKDIR before COPY
* DL3003 use WORKDIR instead of cd
* DL3042 no cache dir for pip
* DL3015 no recommended packages on apt installation
* DL3042 no cache dir for pip
* DL3059 multiple RUN consecutively
* DL3008 pin apt packages version
* SC3014 use = instead of ==
* SC2086 use double quote
* DL3006 the arch must be variable
@akihikokuroda
Copy link
Collaborator

@Tansito FYI: Changed Dockerfiles build images fine on my M1 Mac.

@Tansito
Copy link
Member Author

Tansito commented Jun 19, 2023

Thank you @akihikokuroda ! I was going to ask you for that. I'm try to fix the errors that I couldn't test in my local. I will let you know when I have it

@Tansito Tansito requested a review from IceKhan13 June 19, 2023 17:16
Copy link
Member

@IceKhan13 IceKhan13 left a comment

Choose a reason for hiding this comment

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

🕺

@Tansito
Copy link
Member Author

Tansito commented Jun 19, 2023

@akihikokuroda or @pacomf if you can check now that I finished that I didn't miss anything related with ARM chips it would be awesome 🙏

@akihikokuroda
Copy link
Collaborator

The ray-node image build is failing.

 => ERROR [ray-node-arm64 3/9] RUN apt-get -y update &&    apt-get install --no-install-recommends -y     gcc=4:9.3.0-1ubuntu2     build-essential=12.8ubuntu1     libopenblas-dev=0.3.8+ds-1     cmake=3.16.3-1ubuntu1                  4.3s
------                                                                                                                                                                                                                                        
 > [ray-node-arm64 3/9] RUN apt-get -y update &&    apt-get install --no-install-recommends -y     gcc=4:9.3.0-1ubuntu2     build-essential=12.8ubuntu1     libopenblas-dev=0.3.8+ds-1     cmake=3.16.3-1ubuntu1:                             
#0 0.601 Get:2 http://ports.ubuntu.com/ubuntu-ports focal InRelease [265 kB]                                                                                                                                                                  
#0 0.606 Get:1 https://packages.cloud.google.com/apt kubernetes-xenial InRelease [8993 B]                                                                                                                                                     
#0 0.734 Get:3 https://packages.cloud.google.com/apt kubernetes-xenial/main arm64 Packages [65.8 kB]                                                                                                                                          
#0 1.068 Get:4 http://ports.ubuntu.com/ubuntu-ports focal-updates InRelease [114 kB]                                                                                                                                                          
#0 1.180 Get:5 http://ports.ubuntu.com/ubuntu-ports focal-backports InRelease [108 kB]
#0 1.291 Get:6 http://ports.ubuntu.com/ubuntu-ports focal-security InRelease [114 kB]
#0 1.406 Get:7 http://ports.ubuntu.com/ubuntu-ports focal/multiverse arm64 Packages [139 kB]
#0 1.440 Get:8 http://ports.ubuntu.com/ubuntu-ports focal/main arm64 Packages [1234 kB]
#0 1.637 Get:9 http://ports.ubuntu.com/ubuntu-ports focal/restricted arm64 Packages [1317 B]
#0 1.637 Get:10 http://ports.ubuntu.com/ubuntu-ports focal/universe arm64 Packages [11.1 MB]
#0 2.040 Get:11 http://ports.ubuntu.com/ubuntu-ports focal-updates/universe arm64 Packages [1259 kB]
#0 2.073 Get:12 http://ports.ubuntu.com/ubuntu-ports focal-updates/main arm64 Packages [2429 kB]
#0 2.141 Get:13 http://ports.ubuntu.com/ubuntu-ports focal-updates/restricted arm64 Packages [5450 B]
#0 2.310 Get:14 http://ports.ubuntu.com/ubuntu-ports focal-updates/multiverse arm64 Packages [9072 B]
#0 2.392 Get:15 http://ports.ubuntu.com/ubuntu-ports focal-backports/universe arm64 Packages [27.8 kB]
#0 2.512 Get:16 http://ports.ubuntu.com/ubuntu-ports focal-backports/main arm64 Packages [54.8 kB]
#0 2.604 Get:17 http://ports.ubuntu.com/ubuntu-ports focal-security/restricted arm64 Packages [5214 B]
#0 2.609 Get:18 http://ports.ubuntu.com/ubuntu-ports focal-security/universe arm64 Packages [958 kB]
#0 2.918 Get:19 http://ports.ubuntu.com/ubuntu-ports focal-security/main arm64 Packages [2042 kB]
#0 3.051 Get:20 http://ports.ubuntu.com/ubuntu-ports focal-security/multiverse arm64 Packages [3260 B]
#0 3.139 Fetched 20.0 MB in 3s (7345 kB/s)
#0 3.139 Reading package lists...
#0 3.600 Reading package lists...
#0 4.054 Building dependency tree...
#0 4.148 Reading state information...
#0 4.194 Some packages could not be installed. This may mean that you have
#0 4.194 requested an impossible situation or if you are using the unstable
#0 4.194 distribution that some required packages have not yet been created
#0 4.194 or been moved out of Incoming.
#0 4.194 The following information may help to resolve the situation:
#0 4.194 
#0 4.194 The following packages have unmet dependencies:
#0 4.240  cmake : Depends: cmake-data (= 3.16.3-1ubuntu1) but 3.16.3-1ubuntu1.20.04.1 is to be installed
#0 4.241  libopenblas-dev : Depends: libopenblas0 (= 0.3.8+ds-1) but 0.3.8+ds-1ubuntu0.20.04.1 is to be installed
#0 4.249 E: Unable to correct problems, you have held broken packages.
------
Dockerfile-ray-qiskit:23
--------------------
  22 |     USER $RAY_UID
  23 | >>> RUN apt-get -y update &&\ 
  24 | >>>     apt-get install --no-install-recommends -y \ 
  25 | >>>     gcc=4:9.3.0-1ubuntu2 \ 
  26 | >>>     build-essential=12.8ubuntu1 \
  27 | >>>     libopenblas-dev=0.3.8+ds-1 \
  28 | >>>     cmake=3.16.3-1ubuntu1
  29 |     COPY --chown=$RAY_UID:$RAY_UID ./client ./qs
--------------------
ERROR: failed to solve: process "/bin/sh -c apt-get -y update &&    apt-get install --no-install-recommends -y     gcc=4:9.3.0-1ubuntu2     build-essential=12.8ubuntu1     libopenblas-dev=0.3.8+ds-1     cmake=3.16.3-1ubuntu1" did not complete successfully: exit code: 100

@psschwei
Copy link
Collaborator

@akihikokuroda out of curiosity, do the image builds work if you run docker via colima?

@akihikokuroda
Copy link
Collaborator

@Tansito If I take out the version from these 2 lines. It builds at least. I haven't run using them yet.

    libopenblas-dev=0.3.8+ds-1 \
    cmake=3.16.3-1ubuntu1

@akihikokuroda
Copy link
Collaborator

@psschwei The docker build of the ray-node fails via colima, too. Taking out version from these lines fix the issue via colma, too. It seems some package version conflicts of arm64 packages.

@psschwei
Copy link
Collaborator

🤔 what arch is colima using? colima list should show it. in theory, you should be able to start an x86 environment with colima start --arch x86_64 ... , so wondering if that would make life easier (assuming it works)...

@akihikokuroda
Copy link
Collaborator

@psschwei it is building the image for arm64. The CI build is building building the images for both amd64 and arm64. If it can not build the image for arm64, it fails.

@psschwei
Copy link
Collaborator

I get that. But if colima would let m1 macs run x86 images locally, then maybe we don't have to worry about multi-architecture builds anymore

@akihikokuroda
Copy link
Collaborator

I'm not sure if we can say that colima is the only environment we support for M1 Mac. I think that the multi-arch build should be our direction.

@psschwei
Copy link
Collaborator

My two cents: the end game is to run this on Kubernetes, which at least for IBM Cloud has to be x64. So while arm64 images are nice for local dev, are they going to be run in production? If not, and we have a way to run x86 on m1s, then it seems like a lot of effort to be maintaining two builds...

That said, I'm on an x86 machine, so I'm more than a little biased here 😄 Curious what @pacomf thinks here.

@akihikokuroda
Copy link
Collaborator

My two cents 😄 Some enterprise user may want to run this on their Power or Z System. So it's better for us to have the framework for it.

@pacomf
Copy link
Member

pacomf commented Jun 19, 2023

Agree with @psschwei and @akihikokuroda hahaha.

Right now our goal is the @psschwei thoughts... but we need to be sure that we cant lose the focus about the @akihikokuroda thoughts.

So, we should avoid that it is a real blocker if it is working in some environment like colima for local dev, but we should open issues to track where it is not working, and dedicate some time, when other big priorities are accomplished (like our next release)

@Tansito
Copy link
Member Author

Tansito commented Jun 20, 2023

From what I understood from @akihikokuroda it seems that without those pins the images build so we can start trying to solve that. I'm going to need a couple more days for the next security steps so we have time to investigate it and fix it.

@akihikokuroda if you unpin those libs, builds the image and inspect it, which version those libraries have installed? Thank you for the help!

@akihikokuroda
Copy link
Collaborator

@Tansito It seems that these 2 packages are not necessary to build the pyscf package. We can take out these 2 packages from the install.

@Tansito
Copy link
Member Author

Tansito commented Jun 20, 2023

@akihikokuroda can you try now just to confirm? 🤞

@akihikokuroda
Copy link
Collaborator

@Tansito They are built fine now.

@Tansito
Copy link
Member Author

Tansito commented Jun 20, 2023

Thank you all for checking this, merging now 🙏

@Tansito Tansito merged commit d2e7590 into main Jun 20, 2023
9 checks passed
@Tansito Tansito deleted the docker-lint branch June 20, 2023 15:59
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.

5 participants