Skip to content

Allow dynamic linking of binaries#205

Merged
kubevirt-bot merged 2 commits intokubevirt:mainfrom
awels:generate_dynamic_binaries
Jun 16, 2023
Merged

Allow dynamic linking of binaries#205
kubevirt-bot merged 2 commits intokubevirt:mainfrom
awels:generate_dynamic_binaries

Conversation

@awels
Copy link
Copy Markdown
Member

@awels awels commented Jun 14, 2023

What this PR does / why we need it:
The release was creating statically linked binaries and there is no particular reason for this. Doing dynamically linked binaries would decrase the container size.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Special notes for your reviewer:

Release note:

NONE

@kubevirt-bot kubevirt-bot added release-note-none Denotes a PR that doesn't merit a release note. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Jun 14, 2023
@kubevirt-bot kubevirt-bot requested review from maya-r and mhenriks June 14, 2023 18:24
The release was creating statically linked binaries
and there is no particular reason for this. Doing
dynamically linked binaries would decrase the container
size.

Signed-off-by: Alexander Wels <awels@redhat.com>
@awels awels force-pushed the generate_dynamic_binaries branch from 4442c38 to 2cca599 Compare June 14, 2023 18:27
@aglitke
Copy link
Copy Markdown
Member

aglitke commented Jun 14, 2023

/lgtm
/approve

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Jun 14, 2023
@kubevirt-bot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aglitke

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

The pull request process is described here

Details 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

@kubevirt-bot kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 14, 2023
Added glibc to the Dockerfile so the binaries can run
Fixed failing functional test that was not calculating
the size properly from the `df -Bk` command. It was
calculating from 1000 instead of 1024 and the size was
wrong because of it.

Signed-off-by: Alexander Wels <awels@redhat.com>
@kubevirt-bot kubevirt-bot added size/S and removed lgtm Indicates that a PR is ready to be merged. size/XS labels Jun 15, 2023
@awels
Copy link
Copy Markdown
Member Author

awels commented Jun 15, 2023

@aglitke had to make some adjustments, in particular fix a functional test and adding glibc to the dockerfiles.

@mhenriks
Copy link
Copy Markdown
Member

/lgtm

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Jun 16, 2023
@kubevirt-bot kubevirt-bot merged commit 36fc561 into kubevirt:main Jun 16, 2023
@awels
Copy link
Copy Markdown
Member Author

awels commented Jun 16, 2023

/cherrypick release-v0.16

@awels
Copy link
Copy Markdown
Member Author

awels commented Jun 16, 2023

/cherrypick release-v0.15

@kubevirt-bot
Copy link
Copy Markdown

@awels: new pull request created: #210

Details

In response to this:

/cherrypick release-v0.16

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.

@kubevirt-bot
Copy link
Copy Markdown

@awels: new pull request created: #211

Details

In response to this:

/cherrypick release-v0.15

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.

@awels
Copy link
Copy Markdown
Member Author

awels commented Jun 16, 2023

/cherrypick release-v0.14

@kubevirt-bot
Copy link
Copy Markdown

@awels: new pull request created: #212

Details

In response to this:

/cherrypick release-v0.14

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.

@awels
Copy link
Copy Markdown
Member Author

awels commented Jun 16, 2023

/cherrypick release-v0.13

@kubevirt-bot
Copy link
Copy Markdown

@awels: new pull request created: #213

Details

In response to this:

/cherrypick release-v0.13

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.

@awels
Copy link
Copy Markdown
Member Author

awels commented Jun 16, 2023

/cherrypick release-v0.12

@kubevirt-bot
Copy link
Copy Markdown

@awels: #205 failed to apply on top of branch "release-v0.12":

Applying: Allow dynamic linking of binaries
Applying: Updated fedora to 37 so it has a recent enough glibc Added glibc to the Dockerfile so the binaries can run Fixed failing functional test that was not calculating the size properly from the `df -Bk` command. It was calculating from 1000 instead of 1024 and the size was wrong because of it.
Using index info to reconstruct a base tree...
M	tests/pvc_test.go
Falling back to patching base and 3-way merge...
Auto-merging tests/pvc_test.go
CONFLICT (content): Merge conflict in tests/pvc_test.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0002 Updated fedora to 37 so it has a recent enough glibc Added glibc to the Dockerfile so the binaries can run Fixed failing functional test that was not calculating the size properly from the `df -Bk` command. It was calculating from 1000 instead of 1024 and the size was wrong because of it.
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Details

In response to this:

/cherrypick release-v0.12

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.

awels added a commit to awels/hostpath-provisioner that referenced this pull request Jun 16, 2023
* Allow dynamic linking of binaries

The release was creating statically linked binaries
and there is no particular reason for this. Doing
dynamically linked binaries would decrase the container
size.

Signed-off-by: Alexander Wels <awels@redhat.com>

* Updated fedora to 37 so it has a recent enough glibc
Added glibc to the Dockerfile so the binaries can run
Fixed failing functional test that was not calculating
the size properly from the `df -Bk` command. It was
calculating from 1000 instead of 1024 and the size was
wrong because of it.

Signed-off-by: Alexander Wels <awels@redhat.com>

---------

Signed-off-by: Alexander Wels <awels@redhat.com>
kubevirt-bot pushed a commit that referenced this pull request Jun 20, 2023
* Allow dynamic linking of binaries (#205)

* Allow dynamic linking of binaries

The release was creating statically linked binaries
and there is no particular reason for this. Doing
dynamically linked binaries would decrase the container
size.

Signed-off-by: Alexander Wels <awels@redhat.com>

* Updated fedora to 37 so it has a recent enough glibc
Added glibc to the Dockerfile so the binaries can run
Fixed failing functional test that was not calculating
the size properly from the `df -Bk` command. It was
calculating from 1000 instead of 1024 and the size was
wrong because of it.

Signed-off-by: Alexander Wels <awels@redhat.com>

---------

Signed-off-by: Alexander Wels <awels@redhat.com>

* Use buildah instead of docker, support multi-arch builds (#115)

* Instead of passing environment variables, export them in Makefile

This passes them to all subprocesses, but they can still be overridden
due to the ?= construct.

Signed-off-by: Maya Rashish <mrashish@redhat.com>

* Build sanity.test statically

Avoid failure when the binary is built with Fedora 36 and thus
requires newer glibc symbols than the container.

Signed-off-by: Maya Rashish <mrashish@redhat.com>

* Switch to buildah & podman, enable multi-arch builds

Now we can run:
make clean && \
GOARCH=arm64 make manifest && GOARCH=amd64 make manifest && \
make manifest-push

And spit out a manifest for both arm64 and amd64, in the same image.

Caveats:
- We have a special 'manifest-clean' target, as we can add arbitrarily
  many images to a manifest and don't want the old ones.
  Delete old image in case a regular non-manifest image exists by the
  same name, too.
- The push and image/manifest creation are split, so we can run the
  image creation for more than one architecture and push the combined
  manifest including both.
- We keep `make push` behaving the same to avoid breaking CI.
- Full DOCKER_REPO name is used, as podman-like tools have odd behavior
  with short names.

Signed-off-by: Maya Rashish <mrashish@redhat.com>

* Tolerate docker instead of podman

Signed-off-by: Maya Rashish <mrashish@redhat.com>

* Only add --tls-verify=false if the registry matches localhost*

Add a message about this, too

Signed-off-by: Maya Rashish <mrashish@redhat.com>

* Add go mod vendor

Signed-off-by: Alexander Wels <awels@redhat.com>

---------

Signed-off-by: Alexander Wels <awels@redhat.com>
Signed-off-by: Maya Rashish <mrashish@redhat.com>
Co-authored-by: Maya Rashish <mrashish@redhat.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. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. lgtm Indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants