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

Development image upgraded to ubi8 #1335

Merged
merged 8 commits into from May 24, 2022
Merged

Development image upgraded to ubi8 #1335

merged 8 commits into from May 24, 2022

Conversation

eguzki
Copy link
Member

@eguzki eguzki commented Apr 27, 2022

what

  • Remove dependency from openresty base image from https://github.com/3scale/s2i-openresty
  • Added Dockerfile.devel with the devel image content
  • Fixed spec/resty/openssl/x509_spec.lua unittest. Reason:
    • invalid certificate error message changed due to upgrade on openssl-libs dep:
    • from openssl-libs-1.1.1g-15.el8_3.x86_64 in centos8 -> openssl-libs-1.1.1k-6.el8_5.x86_64 in ubi8
  • Development container had overlapping volume mounts, causing (in linux) to create files with the root owner. The overlapping volume mounts have been removed from docker compose config.

verification steps

run development image

make development

Inside the container, download dependencies

make dependencies

Inside the container, run apicast

$ cat <<EOF >config.json
{
   "services": [
      {
         "proxy": {
             "hosts": ["one"],
             "proxy_rules": [],
             "api_backend": "https://echo-api.3scale.net",
             "policy_chain": []
         }
      }
   ]
}
EOF

$ APICAST_LOG_LEVEL=debug APICAST_WORKER=1 THREESCALE_CONFIG_FILE=config.json BACKEND_ENDPOINT_OVERRIDE=http://localhost:8081 ./bin/apicast

Send request from externally from the host

$ APICAST_IP=$(docker inspect apicast_build_0_development_1 | yq e -P '.[0].NetworkSettings.Networks.apicast_build_0_default.IPAddress' -)

$ curl -v http://${APICAST_IP}:8080
*   Trying 192.168.32.3:8080...
* TCP_NODELAY set
* Connected to 192.168.32.3 (192.168.32.3) port 8080 (#0)
> GET / HTTP/1.1
> Host: 192.168.32.3:8080
> User-Agent: curl/7.68.0
> Accept: */*
> 
* Mark bundle as not supporting multiuse
< HTTP/1.1 404 Not Found
< Server: openresty
< Date: Wed, 27 Apr 2022 16:27:12 GMT
< Content-Type: text/plain
< Transfer-Encoding: chunked
< Connection: keep-alive
< 
* Connection #0 to host 192.168.32.3 left intact

@eguzki eguzki marked this pull request as ready for review April 28, 2022 13:47
@eguzki eguzki requested a review from a team as a code owner April 28, 2022 13:47
@eguzki eguzki requested a review from a team April 28, 2022 13:47
@eguzki eguzki force-pushed the development-image branch 2 times, most recently from d141f78 to 7f5f59c Compare April 28, 2022 16:05
@eguzki eguzki removed the request for review from a team April 29, 2022 08:48
@eguzki eguzki requested a review from a team May 12, 2022 20:04
invalid certificate error message changed due to upgrade on openssl-libs dep
from openssl-libs-1.1.1g-15.el8_3.x86_64 in centos8
to openssl-libs-1.1.1k-6.el8_5.x86_64 in ubi8
Copy link
Member

@kevprice83 kevprice83 left a comment

Choose a reason for hiding this comment

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

A couple of changes I've mentioned but also another question I have is that if this PR is intended to replace the s2i process with a Dockerfile instead shoudln't we be removing all the s2i resources? Or are we still going to rely on those for downstream image builds and use Dockerfile only for upstream?

Tested the general behaviour after fixing the permissions issue and everything seems to be working as expected; integration tests, unit tests and syncing local filesystem changes to container.

Dockerfile.devel Outdated
LUA_CPATH="./lua_modules/lib/lua/5.1/?.so;;" \
LD_LIBRARY_PATH="$LD_LIBRARY_PATH:/opt/app-root/lib"

RUN yum install -y luarocks && \
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be luarocks-${LUAROCKS_VERSION} ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I assumed that it was read by the env var. I did not change from the dockerfile in s2i-openresty repo: https://github.com/3scale/s2i-openresty/blob/96676dbb1a231eb892bb8c2163b3470f6e625527/Dockerfile#L52

I will add it anyway, being explicit is much better.

# no need to access those from docker
- /home/centos/.docker
- /home/centos/.git
working_dir: /opt/app-root/src/
Copy link
Member

Choose a reason for hiding this comment

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

This breaks existing environments because the permissions are not the same, either need to take care of this ourselves OR document that the project should just be recreated.

Copy link
Member Author

Choose a reason for hiding this comment

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

I do not get that about breaking existing environments. The new image "quay.io/3scale/apicast-ci:openresty-1.19.3" or custom image builds from the Dockerfile.devel have HOME set to /opt/app-root/src/ and the running user should not have permission issues.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think I get your point.

If you run "make development", the image being loaded will be quay.io/3scale/apicast-ci:openresty-1.19.3 and there should not be any permission issue.

Only if you run "make development IMAGE=mycustom_image", then you need to make sure that "my_custom_image" is ubi8 based and has been built with "make dev-build" target. Usually the devel image is not custom image. Only when we neeed to upgrade some dep existing in the devel image, we need to test the new devel image with a custom image before pushing a new release

@eguzki
Copy link
Member Author

eguzki commented May 16, 2022

This PR is intended to replace the s2i process with a Dockerfile instead shoudln't we be removing all the s2i resources?

No. This PR is meant to build development image from a local Dockerfile.devel instead of from s2i-openresty repo. Also, upgrading from cento8 to ubi8

Removal of s2i and building upstream runtime image is currently WIP in the following branch: https://github.com/3scale/APIcast/tree/remove-s2i

@eguzki eguzki requested a review from kevprice83 May 19, 2022 13:50
@eguzki eguzki mentioned this pull request May 19, 2022
Copy link
Member

@kevprice83 kevprice83 left a comment

Choose a reason for hiding this comment

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

So after testing again and reproducing the permissions issues it seems like the best solution is to notify the developers (community?) that the new changes require either those dirs to be removed or simply to delete the project and reclone it.

Otherwise nothing else left to review here and the changes look good to me.

@eguzki eguzki merged commit c818ce1 into master May 24, 2022
@eguzki eguzki deleted the development-image branch May 24, 2022 15:40
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.

None yet

2 participants