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

Snap packaging support. #293

Open
wants to merge 29 commits into
base: master
from

Conversation

Projects
None yet
@wwwtyro

wwwtyro commented Mar 22, 2017

This PR adds support for snap packages, fixing #44, for the following kubernetes components:

  • kubefed
  • kubeadm
  • kubectl
  • kube-apiserver
  • kube-controller-manager
  • kube-scheduler
  • kubelet
  • kube-proxy

These follow the same process for deb and RPM by including the appropriate component from the k8s.io build.

@luxas

This comment has been minimized.

Show comment
Hide comment
@luxas

luxas Mar 28, 2017

Member

Thank you for this patch! Adding snap packages to Kubernetes is just great!

I'll take deeper look into this after KubeCon, but I think we should only build packages for kubelet, kubeadm and kubectl. Also we need a package for the CNI binaries.

There should also be snap packages for all available architectures (see the deb process for more info how to easily do that).

cc @mikedanese @jbeda

Member

luxas commented Mar 28, 2017

Thank you for this patch! Adding snap packages to Kubernetes is just great!

I'll take deeper look into this after KubeCon, but I think we should only build packages for kubelet, kubeadm and kubectl. Also we need a package for the CNI binaries.

There should also be snap packages for all available architectures (see the deb process for more info how to easily do that).

cc @mikedanese @jbeda

@castrojo

This comment has been minimized.

Show comment
Hide comment
@castrojo

castrojo commented Apr 3, 2017

wwwtyro and others added some commits Mar 21, 2017

add ceph-common to kube-controller-manager snap (#4)
This is necessary for kube-controller-manager to provision volumes
with `rbd`.
@luxas

Took a super-quick pass and I'm asking myself why there are snaps for apiserver/controller-manager/scheduler/kube-proxy

I don't think those should be debs, the preferred way to run those is in containers.

Re multiarch: You can just download all binaries for all arches from the CI builds -- please look at the deb creation for the specific URLs to use.

No need to compile anything by hand 👍

@marcoceppi

This comment has been minimized.

Show comment
Hide comment
@marcoceppi

marcoceppi Apr 25, 2017

Member

Not everyone runs those as containers, many on-premise users and cloud users in the field run those on the "metal" as services. There's no harm in including them - it's an option.

Member

marcoceppi commented Apr 25, 2017

Not everyone runs those as containers, many on-premise users and cloud users in the field run those on the "metal" as services. There's no harm in including them - it's an option.

@jbeda jbeda assigned marcoceppi and unassigned castrojo Apr 25, 2017

@jbeda

This comment has been minimized.

Show comment
Hide comment
@jbeda

jbeda Apr 25, 2017

Contributor

Assigning to @marcoceppi at the request of @castrojo.

Contributor

jbeda commented Apr 25, 2017

Assigning to @marcoceppi at the request of @castrojo.

@lazypower

This comment has been minimized.

Show comment
Hide comment
@lazypower

lazypower Apr 26, 2017

Member

As this is a known issue moving forward, that we only have AMD64 bins in the snap channels today we have additional incoming feedback requesting PPC64EL. Linking these two issues together so we can continue to track as the status evolves

juju-solutions/bundle-canonical-kubernetes#268

Member

lazypower commented Apr 26, 2017

As this is a known issue moving forward, that we only have AMD64 bins in the snap channels today we have additional incoming feedback requesting PPC64EL. Linking these two issues together so we can continue to track as the status evolves

juju-solutions/bundle-canonical-kubernetes#268

@marcoceppi

This comment has been minimized.

Show comment
Hide comment
@marcoceppi

marcoceppi Apr 26, 2017

Member

I agree, there is a path forward for multi arch builds of snaps, @ryebot @Cynerva is that included in this PR?

Member

marcoceppi commented Apr 26, 2017

I agree, there is a path forward for multi arch builds of snaps, @ryebot @Cynerva is that included in this PR?

@wwwtyro

This comment has been minimized.

Show comment
Hide comment
@wwwtyro

wwwtyro Apr 26, 2017

@marcoceppi It is not; it's here as a PR against this PR branch: juju-solutions#5

It's currently blocked by the issues listed there. I think you indicated you were going to think about how to fix those; could be wrong.

wwwtyro commented Apr 26, 2017

@marcoceppi It is not; it's here as a PR against this PR branch: juju-solutions#5

It's currently blocked by the issues listed there. I think you indicated you were going to think about how to fix those; could be wrong.

@tvansteenburgh

This comment has been minimized.

Show comment
Hide comment
@tvansteenburgh

tvansteenburgh Jun 5, 2017

What's blocking this from being merged, anything?

tvansteenburgh commented Jun 5, 2017

What's blocking this from being merged, anything?

@marcoceppi

This comment has been minimized.

Show comment
Hide comment
@marcoceppi

marcoceppi Jun 5, 2017

Member

I think this is ready for review again? Is that correct @wwwtyro @Cynerva @chuckbutler ?

Member

marcoceppi commented Jun 5, 2017

I think this is ready for review again? Is that correct @wwwtyro @Cynerva @chuckbutler ?

@wwwtyro

This comment has been minimized.

Show comment
Hide comment
@wwwtyro

wwwtyro Jun 5, 2017

@tvansteenburgh @marcoceppi Last I touched this, we hadn't quite finished the cross-platform snap support, which is what I think was blocking this.

wwwtyro commented Jun 5, 2017

@tvansteenburgh @marcoceppi Last I touched this, we hadn't quite finished the cross-platform snap support, which is what I think was blocking this.

@tvansteenburgh

This comment has been minimized.

Show comment
Hide comment
@tvansteenburgh

tvansteenburgh Jun 5, 2017

@wwwtyro Can you summarize what's left to do?

tvansteenburgh commented Jun 5, 2017

@wwwtyro Can you summarize what's left to do?

@wwwtyro

This comment has been minimized.

Show comment
Hide comment
@wwwtyro

wwwtyro Jun 5, 2017

@tvansteenburgh I think we're mostly blocked by this and need it fixed or a workaround found: https://bugs.launchpad.net/snapcraft/+bug/1686481

wwwtyro commented Jun 5, 2017

@tvansteenburgh I think we're mostly blocked by this and need it fixed or a workaround found: https://bugs.launchpad.net/snapcraft/+bug/1686481

@dawidmalina

This comment has been minimized.

Show comment
Hide comment
@dawidmalina

dawidmalina Aug 24, 2017

Any updates on this? Maybe we could postpone multiarch suport and go with what we already have (more agile - step by step) instead blocking this pull request?

dawidmalina commented Aug 24, 2017

Any updates on this? Maybe we could postpone multiarch suport and go with what we already have (more agile - step by step) instead blocking this pull request?

@luxas

This comment has been minimized.

Show comment
Hide comment
@luxas

luxas Aug 26, 2017

Member

What's blocking multiarch support for the snaps?

Member

luxas commented Aug 26, 2017

What's blocking multiarch support for the snaps?

@NickZ

This comment has been minimized.

Show comment
Hide comment
@NickZ

NickZ Sep 9, 2017

@luxas it appears to be an upstream bug; snapcraft will not build packages for anything other than the current architecture, even if the architecture is listed in the snapcraft.yaml.

https://bugs.launchpad.net/snapcraft/+bug/1686481?comments=all

Unfortunately, I don't think there's much to be done for multiarch support until upstream fixes this.

NickZ commented Sep 9, 2017

@luxas it appears to be an upstream bug; snapcraft will not build packages for anything other than the current architecture, even if the architecture is listed in the snapcraft.yaml.

https://bugs.launchpad.net/snapcraft/+bug/1686481?comments=all

Unfortunately, I don't think there's much to be done for multiarch support until upstream fixes this.

@k8s-ci-robot k8s-ci-robot added the size/L label Mar 2, 2018

@k8s-ci-robot k8s-ci-robot added size/XL and removed size/L labels Mar 29, 2018

Cynerva and others added some commits Mar 29, 2018

Stop building kubefed (#17)
* Stop building kubefed

* Adding kubefed.yaml back
Source /root/cdk/kube.env in daemon wrappers (#19)
In order to pass env vars to the daemons in a generic fashion, this
sources the /root/cdk/kube.env file if it exists.
support building snap variants (#21)
We need to be able to build snaps with a suffix. These are identical
to our normal snap builds, but allow us to have a variation in
the store, like 'kubectl-foo'. This makes it possible to keep variants
on different versions without having a bunch of channels muddying up
the primary 'kubectl' snap.
@dims

This comment has been minimized.

Show comment
Hide comment
@dims

dims Jul 3, 2018

Member

@luxas is this still relevant?

Member

dims commented Jul 3, 2018

@luxas is this still relevant?

@luxas

This comment has been minimized.

Show comment
Hide comment
@luxas

luxas Jul 4, 2018

Member

Not for me personally at least. I think we're moving towards containers for the control plane & debs/rpms for the kubelet, kubeadm and kubectl, built automatically with bazel

Member

luxas commented Jul 4, 2018

Not for me personally at least. I think we're moving towards containers for the control plane & debs/rpms for the kubelet, kubeadm and kubectl, built automatically with bazel

@k8s-ci-robot

This comment has been minimized.

Show comment
Hide comment
@k8s-ci-robot

k8s-ci-robot Aug 8, 2018

Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: wwwtyro
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: jberkus

If they are not already assigned, you can assign the PR to them by writing /assign @jberkus in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Contributor

k8s-ci-robot commented Aug 8, 2018

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: wwwtyro
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: jberkus

If they are not already assigned, you can assign the PR to them by writing /assign @jberkus in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@calebamiles

This comment has been minimized.

Show comment
Hide comment
@calebamiles

calebamiles Sep 5, 2018

Member

/hold

where/how snaps should be built and hosted is probably a larger discussion than a single PR, but thanks a lot for the work!

Member

calebamiles commented Sep 5, 2018

/hold

where/how snaps should be built and hosted is probably a larger discussion than a single PR, but thanks a lot for the work!

Add snapcraft cleanbuild (#28)
This makes sure snaps can still be built on bionic hosts.

Signed-off-by: Adam Stokes <battlemidget@users.noreply.github.com>
@k8s-ci-robot

This comment has been minimized.

Show comment
Hide comment
@k8s-ci-robot

k8s-ci-robot Sep 11, 2018

Contributor

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.


Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

Contributor

k8s-ci-robot commented Sep 11, 2018

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.


Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

battlemidget and others added some commits Sep 12, 2018

Fix docker builds (#29)
* Fix docker builds

This updates to a recent stable container for building snaps via docker.

Signed-off-by: Adam Stokes <battlemidget@users.noreply.github.com>

* Include file in container, link magic db

Signed-off-by: Adam Stokes <battlemidget@users.noreply.github.com>
Update Makefile to use docker-build (#30)
Signed-off-by: Adam Stokes <battlemidget@users.noreply.github.com>
Fix KUBE_VERSION and KUBE_ARCH being ignored (#32)
@battlemidget Looks like somewhere along the way we lost the ability to pass KUBE_VERSION and KUBE_ARCH through to the build script. It just always builds the latest stable Kubernetes (currently v1.11.3). This PR should fix it.

I also moved the docker-build.sh script into build-scripts/ to tidy up a bit, hope that's cool.
Add Dockerfile and update build script to use that (#33)
Run with:

cd release/snap
KUBE_ARCH=amd64 KUBE_VERSION=v1.11.3 ./build-scripts/docker-build kubectl

Signed-off-by: Adam Stokes <battlemidget@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment