-
Notifications
You must be signed in to change notification settings - Fork 502
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
Bundle CNI plugins (v0.8.6) in kubelet deb/rpm packages #1309
Bundle CNI plugins (v0.8.6) in kubelet deb/rpm packages #1309
Conversation
7e07bc0
to
8d3a79a
Compare
c20a749
to
b4114ee
Compare
/test pull-release-cluster-up |
b4114ee
to
23ec64e
Compare
debs seem to be fine, but the rpms are busted, so I'll keep experimenting... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If possible in both RPM and Deb should give metadata cookie crumb to avoid kubernetes-cni and the new bundled kubelet package at the same time.
Source6: kubelet.env | ||
Source7: https://github.com/kubernetes-sigs/cri-tools/releases/download/v%{CRI_TOOLS_VERSION}/crictl-v%{CRI_TOOLS_VERSION}-linux-%{ARCH}.tar.gz | ||
|
||
BuildRequires: systemd | ||
BuildRequires: curl | ||
Requires: iptables >= 1.4.21 | ||
Requires: kubernetes-cni >= %{CNI_VERSION} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably change from Requires: to Obsoletes:, eg:
Obsoletes: kubernetes-cni
I don't know the debian meta options. Is there something like this there too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Tim! I'm going to do a little research and come back with the deb equivalent as well next week.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tpepper -- This should be ready to merge now. See the worklog below: #1309 (comment)
I ended up adding this to the rpm spec:
Obsoletes: kubernetes-cni
Conflicts: kubernetes-cni
and this to the debian package config:
Provides: kubernetes-cni
Conflicts: kubernetes-cni
Replaces: kubernetes-cni
Signed-off-by: Stephen Augustus <saugustus@vmware.com>
Signed-off-by: Stephen Augustus <saugustus@vmware.com>
Signed-off-by: Stephen Augustus <saugustus@vmware.com>
52e2877
to
7ca71bc
Compare
Testing... Generate new packagesdebsFrom $ time go run build.go -arch amd64 -distros bionic -kube-version 1.18.3 -revision 01 rpmsFrom $ time ./docker-build.sh Use test install scripts for debs/rpms (from k/sig-release/packaging.md)debs#!/usr/bin/env bash
version="$1"
[[ -n "${version}" ]] \
&& curl -s https://packages.cloud.google.com/apt/doc/apt-key.gpg \
| sudo apt-key add - \
&& echo "deb http://apt.kubernetes.io/ kubernetes-xenial main" \
| sudo tee /etc/apt/sources.list.d/kubernetes.list \
&& sudo apt-get update -q \
&& sudo apt-get install -qy kubelet="${version}-00" kubectl="${version}-00" kubeadm="${version}-00" rpms#!/usr/bin/env bash
version="$1"
if [[ -n "${version}" ]]; then
cat <<EOF | sudo tee /etc/yum.repos.d/kubernetes.repo
[kubernetes]
name=Kubernetes
baseurl=https://packages.cloud.google.com/yum/repos/kubernetes-el7-x86_64
enabled=1
gpgcheck=1
repo_gpgcheck=1
gpgkey=https://packages.cloud.google.com/yum/doc/yum-key.gpg https://packages.cloud.google.com/yum/doc/rpm-package-key.gpg
EOF
sudo yum install -y kubelet-${version} kubeadm-${version} kubectl-${version} --disableexcludes=kubernetes
fi debs testinginstall upstream debs
Create update-debs.sh#!/usr/bin/env bash
cd /k-release/debian/bin/stable/bionic/
dpkg-scanpackages . /dev/null | gzip -9c > Packages.gz Update the package list to scan local debs
Add the following line:
Install updated kubeadm and kubelet packages
Check package configurations
rpms testinginstall upstream rpms
Install updated kubeadm and kubelet packages
Check package configurations
|
Ready for approval. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: justaugustus, saschagrunert 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 |
mv cni-plugins/* %{buildroot}/opt/cni/bin/ | ||
|
||
%files | ||
%{_bindir}/kubelet | ||
%{_unitdir}/kubelet.service | ||
%{_sysconfdir}/kubernetes/manifests/ | ||
/opt/cni |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure about the implications but I think we should not own files in /opt
generally in RPMs. Do you think we could move it to _libexecdir
?
I don't know much about RPMs, but I have a suspicion this broke everybody. |
This is now being bundled in the `kubelet` package, see: kubernetes/release#1309.
This is now being bundled in the `kubelet` package, see: kubernetes/release#1309.
This is now being bundled in the `kubelet` package, see: kubernetes/release#1309.
This is now being bundled in the `kubelet` package, see: kubernetes/release#1309.
What type of PR is this?
/kind cleanup
/area dependency
/sig network
What this PR does / why we need it:
Bundle CNI plugins (v0.8.6) in kubelet deb/rpm packages
(Supports k/k CNI plugin update: kubernetes/kubernetes#91370)
ref: #885
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?