Skip to content

Conversation

@mboersma
Copy link
Member

@mboersma mboersma commented Dec 1, 2016

A Ubuntu medium CVE patch broke docker-py behavior in an odd way that wasn't easily fixed. Switching dockerbuilder to python3 seems to have avoided the damage.

This fix could use thorough reviewing and testing in addition to CI. Especially WRT the string vs. byte differences introduced by Python 3.

@deis-bot
Copy link

deis-bot commented Dec 1, 2016

@helgi, @krancour and @kmala are potential reviewers of this pull request based on my analysis of git blame information. Thanks @mboersma!

@bacongobbler
Copy link
Member

tested this image manually with example-dockerfile-python and it ran to completion. LGTM 👍

@bacongobbler
Copy link
Member

bacongobbler commented Dec 1, 2016

Sample output:

><> git push deis master
Counting objects: 64, done.
Delta compression using up to 8 threads.
Compressing objects: 100% (47/47), done.
Writing objects: 100% (64/64), 11.75 KiB | 0 bytes/s, done.
Total 64 (delta 15), reused 64 (delta 15)
Starting build... but first, coffee!
...
Step 1 : FROM gliderlabs/alpine:3.4
---> a5e834e13f19
Step 2 : RUN apk-install python
---> Using cache
---> 041cc7c5b589
Step 3 : ADD . /app
---> Using cache
---> 966d73b9370e
Step 4 : WORKDIR /app
---> Using cache
---> a2d85f4ddfde
Step 5 : CMD python -m SimpleHTTPServer 5000
---> Using cache
---> ae5e8561a6fc
Step 6 : EXPOSE 5000
---> Using cache
---> 17fc4217a113
Successfully built 17fc4217a113
Pushing to registry
Build complete.
Launching App...
...
Done, py:v2 deployed to Workflow

Use 'deis open' to view this application in your browser

To learn more, use 'deis help' or visit https://deis.com/

To ssh://git@deis-builder.k8s.local:2222/py.git
 * [new branch]      master -> master
><> kd get po dockerbuild-py-77449891-c631b0ed -o yaml | grep "image:"
    image: quay.io/deisci/dockerbuilder:git-5a76326
    image: quay.io/deisci/dockerbuilder:git-5a76326

@bacongobbler
Copy link
Member

Oddly this failed on CI though:

11:35:34 remote: ---> Starting build... but first, coffee!        
11:35:34 remote: Step 1 : FROM alpine:3.3        
11:35:34 remote: ---> 6c2aa2137d97        
11:35:34 remote: Step 2 : RUN apk add -U 	bash 	nginx 	&& rm -rf /var/cache/apk*        
11:35:34 remote: ---> Using cache        
11:35:34 remote: ---> daf48dd65de3        
11:35:34 remote: Step 3 : RUN ln -sf /dev/stdout /var/log/nginx/access.log        
11:35:34 remote: ---> Using cache        
11:35:34 remote: ---> 056a79dfd1ff        
11:35:34 remote: Step 4 : RUN ln -sf /dev/stderr /var/log/nginx/error.log        
11:35:34 remote: ---> Using cache        
11:35:34 remote: ---> 1b9e99e78554        
11:35:34 remote: Traceback (most recent call last):        
11:35:34 remote:   File "/deploy.py", line 103, in <module>        
11:35:34 remote:     log_output(stream, True)        
11:35:34 remote:   File "/deploy.py", line 25, in log_output        
11:35:34 remote:     print(stream_chunk.replace(b'\n', b'').decode('utf-8'))        
11:35:34 remote: UnicodeEncodeError: 'ascii' codec can't encode characters in position 18-27: ordinal not in range(128) 

@mboersma
Copy link
Member Author

mboersma commented Dec 1, 2016

Clearly this needs some more attention to string handling and failure cases, but at least the happy path is happy again.

@mboersma mboersma force-pushed the hotfix-ubuntu-damage branch from 5a76326 to 39bd9bd Compare December 1, 2016 22:43
rootfs/deploy.py Outdated
if stream_chunk:
stream_chunk = stream_chunk.encode('utf-8').strip()
print(stream_chunk.replace('\n', ''))
# Must handle chunks as bytes to avoid UnicdoeEncodeError.
Copy link
Member

Choose a reason for hiding this comment

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

typo: UnicdoeEncodeError should be UnicodeEncodeError

@bacongobbler
Copy link
Member

woops, looks like I double-LGTM'd this. Let me revert that lol

@mboersma mboersma force-pushed the hotfix-ubuntu-damage branch from 39bd9bd to 0b6d8a9 Compare December 1, 2016 23:07
@mboersma mboersma merged commit 463b537 into deis:master Dec 1, 2016
@mboersma mboersma deleted the hotfix-ubuntu-damage branch December 1, 2016 23:07
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.

5 participants