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
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.
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.
(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.
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 ?
Also the same for iptables and https://github.com/jasonbrooks/k8s-images/blob/fedpkg/proxy/Dockerfile
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 ?)
(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.
(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.
(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.
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.
(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.
(and I also sponsored Jason to the packager group)
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/docker/kubernetes
(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.
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/docker/kubernetes-kubelet
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/docker/kubernetes-node