Skip to content
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

switch off of the bazel debs #335

Merged
merged 1 commit into from
Feb 23, 2019

Conversation

BenTheElder
Copy link
Member

easiest answer to fixing the bazel build after cross compliation landed.
/hold

positive:

  • the bazel build and the non-bazel build have the same install process now
  • we can probably refactor the build to only COPY operations and support cross-compilation Support arm64 #166
  • no longer broken by changes to the debs, the binaries are less in-flux

negative:

  • no longer installing the debs

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 23, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: BenTheElder

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

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Feb 23, 2019
@k8s-ci-robot k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Feb 23, 2019
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 23, 2019
@BenTheElder
Copy link
Member Author

all the builds passed this time, pushing a slight cleanup / refactor

@BenTheElder
Copy link
Member Author

/hold cancel
/kind bug
/priority critical-urgent

@k8s-ci-robot k8s-ci-robot added kind/bug Categorizes issue or PR as related to a bug. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Feb 23, 2019
if err := install.Run("/bin/sh", "-c", "dpkg -i "+debs); err != nil {
log.Errorf("Debian install failed! %v", err)
return err
kindBinDir := path.Join(install.BasePath(), "bin")
Copy link
Member Author

@BenTheElder BenTheElder Feb 23, 2019

Choose a reason for hiding this comment

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

this is the install logic from the make/bash/docker build, they match now

filepath.Join(buildDir, "debs", "kubelet.deb"): "debs/kubelet.deb",
filepath.Join(buildDir, "debs", "kubectl.deb"): "debs/kubectl.deb",
filepath.Join(buildDir, "debs", "kubernetes-cni.deb"): "debs/kubernetes-cni.deb",
filepath.Join(buildDir, "debs", "cri-tools.deb"): "debs/cri-tools.deb",
Copy link
Member

Choose a reason for hiding this comment

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

How are cri-tools and kubernetes-cni now installed? Don't see any references to them elsewhere 🙄

Copy link
Member Author

Choose a reason for hiding this comment

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

they're installed in the base image, this is now on par with the make/bash/docker build.

these debs just wrap downloading a copy of a stable release of these based on WORKSPACE, we do ~the same in the base image already.

Copy link
Member Author

Choose a reason for hiding this comment

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

or rather CNI is:

# Install CNI binaries to /opt/cni/bin
# TODO(bentheelder): doc why / what here
ARG CNI_VERSION="0.6.0"
ARG CNI_TARBALL="cni-plugins-${ARCH}-v${CNI_VERSION}.tgz"
ARG CNI_BASE_URL="https://storage.googleapis.com/kubernetes-release/network-plugins/"
ARG CNI_URL="${CNI_BASE_URL}${CNI_TARBALL}"
RUN curl -sSL --retry 5 --output /tmp/cni.tgz "${CNI_URL}" \
&& sha256sum /tmp/cni.tgz \
&& mkdir -p /opt/cni/bin \
&& tar -C /opt/cni/bin -xzf /tmp/cni.tgz \
&& rm -rf /tmp/cni.tgz

but cri-tools was only to satisfy the deb install requirements, we don't use those while we're on the dockershim path.

Copy link
Member Author

@BenTheElder BenTheElder Feb 23, 2019

Choose a reason for hiding this comment

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

if/when we take a poke at CRI path we can install these to the base image, they have stable releases and don't change often in the kubernetes WORKSPACE, and IIRC are backwards/forwards compatible.

@munnerz
Copy link
Member

munnerz commented Feb 23, 2019

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 23, 2019
@tao12345666333
Copy link
Member

/lgtm

@BenTheElder
Copy link
Member Author

all of the builds passed again, so we're just waiting for the e2es 😅

@k8s-ci-robot k8s-ci-robot merged commit c64da77 into kubernetes-sigs:master Feb 23, 2019
@BenTheElder BenTheElder deleted the fix-the-build branch February 23, 2019 14:54
stg-0 pushed a commit to stg-0/kind that referenced this pull request Feb 6, 2024
* version v0.18.0-alpha

* update docs for v0.17.0

* fix kind version in readme

* comments-update-buildcontext

* Added validations for azs, k8sVersion, vpcs and subnets in each provider

* fixing dependency bugs

* Merge with master

* fixed gcp

* Update pkg/cluster/internal/validate/gcp.go

Co-authored-by: esierra-stratio <102975650+esierra-stratio@users.noreply.github.com>

* fixed build

* Update CHANGELOG.md

---------

Co-authored-by: Benjamin Elder <bentheelder@google.com>
Co-authored-by: Daman <aroradaman@gmail.com>
Co-authored-by: Kubernetes Prow Robot <k8s-ci-robot@users.noreply.github.com>
Co-authored-by: esierra-stratio <102975650+esierra-stratio@users.noreply.github.com>
stg-0 pushed a commit to stg-0/kind that referenced this pull request Feb 6, 2024
* chore: bump azure provider

* bump azure provider version

* [CLOUD-109] Integrar GKE como nuevo provider (kubernetes-sigs#320)

* Initial gke support

* Fix gcr authentication

* Minor fixes

* Fix gcr authentication

* Enable machine pools for gke

* Enable gke in capg

* Add google artifact registry support

* Add gke version validation

* Improve error message

* Update cluster-operator version

* Fix gke install logic

* Improve execute command retries (kubernetes-sigs#298)

* Improve execute command retries

* Fix kubeconfig condition

* Fix ecr login when ecr is in another region

* Remove comment

* [BUILD] 0.17.0-0.3.0: New pre-release

* Prepare for next version: 0.17.0-0.3.0-SNAPSHOT

* Prepare for next version: 0.17.0-0.3.1-SNAPSHOT

* Improve retry logic

---------

Co-authored-by: stratiocommit <stratiocommit@stratio.com>

* Feature/add infra validations (kubernetes-sigs#335)

* version v0.18.0-alpha

* update docs for v0.17.0

* fix kind version in readme

* comments-update-buildcontext

* Added validations for azs, k8sVersion, vpcs and subnets in each provider

* fixing dependency bugs

* Merge with master

* fixed gcp

* Update pkg/cluster/internal/validate/gcp.go

Co-authored-by: esierra-stratio <102975650+esierra-stratio@users.noreply.github.com>

* fixed build

* Update CHANGELOG.md

---------

Co-authored-by: Benjamin Elder <bentheelder@google.com>
Co-authored-by: Daman <aroradaman@gmail.com>
Co-authored-by: Kubernetes Prow Robot <k8s-ci-robot@users.noreply.github.com>
Co-authored-by: esierra-stratio <102975650+esierra-stratio@users.noreply.github.com>

* version v0.18.0-alpha

* delete unneeded files

* add CHANGELOG

---------

Co-authored-by: Francisco Augusto <faugusto@stratio.com>
Co-authored-by: stratiocommit <stratiocommit@stratio.com>
Co-authored-by: lreciomelero <120394823+lreciomelero@users.noreply.github.com>
Co-authored-by: Benjamin Elder <bentheelder@google.com>
Co-authored-by: Daman <aroradaman@gmail.com>
Co-authored-by: Kubernetes Prow Robot <k8s-ci-robot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants