Bug 1404421 - Container Review Request: kubernetes - containers for kubernetes master and node packages
Summary: Container Review Request: kubernetes - containers for kubernetes master and n...
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Michael S.
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2016-12-13 19:56 UTC by Jason Brooks
Modified: 2020-04-30 14:00 UTC (History)
4 users (show)

Fixed In Version: kubernetes-0.1-1.f25docker
Clone Of:
Environment:
Last Closed: 2020-04-30 14:00:37 UTC
Type: ---
Embargoed:
misc: fedora-review+


Attachments (Terms of Use)

Description Jason Brooks 2016-12-13 19:56:50 UTC
Dockerfile(s) URL: https://github.com/jasonbrooks/k8s-images/tree/fedpkg
Description: Kubernetes containers for fedora, based originally on https://github.com/fedora-cloud/Fedora-Dockerfiles/pull/112
Fedora Account System Username: jasonbrooks

Comment 1 Michael S. 2016-12-13 21:49:09 UTC
So, I am wondering if the review should cover all of them at once, or if we should them one by one. Fedora rpm do them one by one, and that's likely easier to follow with 1 bug, 1 review and 1 container.

But that's also subpackage of the same source so I guess there is some reason.

Comment 2 Michael S. 2016-12-13 22:05:35 UTC
Ok, so the master seems to be good.

There is the required labels, there is no extra code. 

I just have reservation regarding Maintainer, but that's not in the guideline for now.

Comment 3 Jason Brooks 2016-12-13 22:08:14 UTC
(In reply to Michael Scherer from comment #2)
> Ok, so the master seems to be good.
> 
> There is the required labels, there is no extra code. 
> 
> I just have reservation regarding Maintainer, but that's not in the
> guideline for now.

I can put myself in there as the maintainer, if that's better.

Comment 4 Michael S. 2016-12-13 22:15:11 UTC
Ok so:
- why does kubelet install utils-linux ?

( https://github.com/jasonbrooks/k8s-images/blob/fedpkg/kubelet/Dockerfile#L11 )
the script do not seems to use it, and so, if kubelet use it,maybe the deps should be in the rpm ?

Comment 5 Michael S. 2016-12-13 22:15:44 UTC
Also the same for iptables and https://github.com/jasonbrooks/k8s-images/blob/fedpkg/proxy/Dockerfile

Comment 6 Michael S. 2016-12-13 22:20:15 UTC
Other than this, they do look ok, even if I do not have enough knowledge regarding the kubernetes parameters to see if the scripts are correct. I also wonder if the proxy image shouldn't get more documention on how to run it (IIRC? it is supposed to forward various ports around, so we need to use -p, no ?)

Comment 7 Jason Brooks 2016-12-14 03:02:40 UTC
(In reply to Michael Scherer from comment #4)
> Ok so:
> - why does kubelet install utils-linux ?
> 
> (
> https://github.com/jasonbrooks/k8s-images/blob/fedpkg/kubelet/Dockerfile#L11
> )
> the script do not seems to use it, and so, if kubelet use it,maybe the deps
> should be in the rpm ?

The kubelet, when containerized, needs nsenter, which is provided by utils-linux.

Comment 8 Jason Brooks 2016-12-14 03:03:32 UTC
(In reply to Michael Scherer from comment #5)
> Also the same for iptables and
> https://github.com/jasonbrooks/k8s-images/blob/fedpkg/proxy/Dockerfile

iptables is already pulled in by kubernetes-node, so I removed it from the Dockerfile.

Comment 9 Jason Brooks 2016-12-14 03:11:03 UTC
(In reply to Michael Scherer from comment #6)
> Other than this, they do look ok, even if I do not have enough knowledge
> regarding the kubernetes parameters to see if the scripts are correct. I
> also wonder if the proxy image shouldn't get more documention on how to run
> it (IIRC? it is supposed to forward various ports around, so we need to use
> -p, no ?)

I've been adapting the ansible scripts from kubernetes/contrib to use these packages. I'm working in this fork: https://github.com/jasonbrooks/contrib/tree/atomic-update

I found that I needed to add the -p 443 for the apiserver component, and the proxy component didn't need a similar change to work with these ansible scripts, but the scripts do handle firewall configuration.

I've worked through some steps to run the node portion with package layering and the master parts in containers in this gist: https://gist.github.com/jasonbrooks/f1aa092e63edce5272451c5845f72750. I could look into what other changes might be needed to make that ansibleless method work with the kubelet and proxy in containers, too.

Comment 10 Michael S. 2016-12-14 17:12:08 UTC
Ok so I guess that's ok for me. I would like to make sure that any time we install a package, we justifiy (like a comment), but that should be blocking.

Comment 11 Jason Brooks 2016-12-14 17:18:29 UTC
(In reply to Michael Scherer from comment #10)
> Ok so I guess that's ok for me. I would like to make sure that any time we
> install a package, we justifiy (like a comment), but that should be blocking.

I added a comment for the utils-linux justification.

Comment 12 Michael S. 2016-12-14 17:18:47 UTC
(and I also sponsored Jason to the packager group)

Comment 13 Gwyn Ciesla 2017-01-04 17:57:24 UTC
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/docker/kubernetes

Comment 14 Jason Brooks 2017-01-05 22:02:49 UTC
(In reply to Jon Ciesla from comment #13)
> Package request has been approved:
> https://admin.fedoraproject.org/pkgdb/package/docker/kubernetes

It looks like I need to submit the 7 sub-images for kubernetes as separate packages, I'm going to reference this bz in each of them, and then delete the plain "docker/kubernetes". Let me know if that's a problem.

Comment 15 Gwyn Ciesla 2017-01-06 14:57:04 UTC
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/docker/kubernetes-kubelet

Comment 16 Gwyn Ciesla 2017-01-06 15:06:04 UTC
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/docker/kubernetes-node


Note You need to log in before you can comment on or make changes to this bug.