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

Bug 1838512: Set CGO_ENABLED=1 for cross compile on Mac #644

Closed
wants to merge 2 commits into from

Conversation

damemi
Copy link

@damemi damemi commented Nov 17, 2020

With the changes in openshift-eng/ocp-build-data#627, C cross-compilers should be available in our build images. Enabling this for darwin builds of oc would fix the referenced bug, in which cgo is required for proper dns resolution on mac (and was fixed in upstream kubectl long ago: kubernetes/release#469)

@openshift-ci-robot openshift-ci-robot added bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. labels Nov 17, 2020
@openshift-ci-robot
Copy link

@damemi: This pull request references Bugzilla bug 1838512, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.7.0) matches configured target release for branch (4.7.0)
  • bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1838512: Set CGO_ENABLED=1 for cross compile on Mac

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.

@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: damemi
To complete the pull request process, please assign smarterclayton after the PR has been reviewed.
You can assign the PR to them by writing /assign @smarterclayton in a comment when ready.

The full list of commands accepted by this bot can be found 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

@damemi
Copy link
Author

damemi commented Nov 17, 2020

/cc @soltysh
@ecordell and @sosiouxme, could you also please take a look and let us know if there is anything else we need to do to take advantage of the changes in openshift-eng/ocp-build-data#627? Thanks!

@openshift-merge-robot
Copy link
Contributor

@damemi: The following tests failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/rpm-build 9b81908 link /test rpm-build
ci/prow/e2e-aws 9b81908 link /test e2e-aws
ci/prow/e2e-agnostic-cmd 9b81908 link /test e2e-agnostic-cmd
ci/prow/e2e-aws-serial 9b81908 link /test e2e-aws-serial
ci/prow/images 9b81908 link /test images
ci/prow/build-rpms-from-tar 9b81908 link /test build-rpms-from-tar
ci/prow/e2e-aws-upgrade 9b81908 link /test e2e-aws-upgrade

Full PR test history. Your PR dashboard.

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.

@soltysh
Copy link
Contributor

soltysh commented Nov 18, 2020

@damemi looks like this is breaking CI pretty badly

@yselkowitz
Copy link
Contributor

This cannot be merged right now at least:

  1. Dynamic linking of darwin binaries is currently broken in 1.15 and currently only work with 1.14; that should be fixed in 1.15.4 but we don't have that yet.
  2. Cross compilers for darwin binaries are only available on the x86_64 golang-builder, so such a build will fail on other architectures.
  3. In order to use the cross-compilers, CC, CXX, and -extld need to be declared.

/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 18, 2020
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 15, 2021

@damemi: The following tests failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/rpm-build f730977 link /test rpm-build
ci/prow/build-rpms-from-tar f730977 link /test build-rpms-from-tar
ci/prow/e2e-metal-ipi-ovn-ipv6 f730977 link /test e2e-metal-ipi-ovn-ipv6
ci/prow/e2e-aws f730977 link /test e2e-aws
ci/prow/e2e-agnostic-cmd f730977 link /test e2e-agnostic-cmd
ci/prow/images f730977 link /test images
ci/prow/e2e-aws-serial f730977 link /test e2e-aws-serial
ci/prow/e2e-aws-upgrade f730977 link /test e2e-aws-upgrade

Full PR test history. Your PR dashboard.

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.

@yselkowitz
Copy link
Contributor

golang-builder 1.15 can now build Mac binaries, but the second point above still applies: only the x86_64 golang-builder has the means to dynamically build Mac binaries, so this will break the cli-artifacts build on other architectures. Furthermore, this will break the RPM build entirely, as the RPM buildroot is not set up to build Mac binaries either for any architecture. This needs further coordination before it can be considered.

@damemi
Copy link
Author

damemi commented Feb 25, 2021

@yselkowitz to check up on this, are the limitations you pointed out something that the oc team can work to resolve? Or is this something that is just not currently possible to accomplish with our build and release systems?

I just ask because the linked bug has been open for quite some time, and I would like to determine whether we have the ability to move forward with this or it should be closed. Thanks!

@sosiouxme
Copy link
Member

@damemi So the two problems:

only the x86_64 golang-builder has the means to dynamically build Mac binaries, so this will break the cli-artifacts build on other architectures.

This is something the oc team can work on; we had a similar situation getting opm to build in operator-registry and it required some minor kludges because Dockerfile doesn't provide logic for optional contents. Everything has to be scripted to account for toolchains and outputs that exist on one architecture and not others -- the cross-compile toolchain just literally does not exist in RH for non-x86 arches.

Furthermore, this will break the RPM build entirely, as the RPM buildroot is not set up to build Mac binaries either for any architecture.

RPMs build in a completely different environment than an image; actually, in the case of oc, two different environments - it has to compile against both RHEL 7 and RHEL 8 buildroots. Both of those buildroots would need to have the necessary toolchain available (this would be something ART has to investigate; not even sure how to indicate BuildRequires for a subset of arches, and we have to involve EXD to configure brew), and again the RPM builds need logic to account for the part of the build that only runs on x86_64 (something for the oc team).

So I think I can see a path for this to happen, but it sure sounds like a lot of work.

@damemi
Copy link
Author

damemi commented Mar 4, 2021

@sosiouxme thank you for the detailed response. That does seem like a lot of work, so maybe we need to do a cost-benefit analysis of this effort.

The linked BZ describes that the oc binary we ship for mac is completely unusable when behind certain corporate VPNs. Because this only affects mac, on some VPN configurations, this is a subset of our users. This is still a very bad look, however I do not know if the size of that subset is significant enough for us to devote the work you said to fix it.

@soltysh what should we do with this? Users that are affected do have the option of compiling oc themselves, but that's not really a solution. Do we have some way to measure what % of users are mac, and what % of those are using OpenShift with a VPN?

I am willing to continue driving this work if necessary, but the honest opinion I seem to get from the relevant build teams is that this is difficult, or impossible, to fix with our current build tooling. I don't want to push on something this complex if it outweighs the need to provide a fix.

@openshift-bot
Copy link
Contributor

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci openshift-ci bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 3, 2021
@yselkowitz
Copy link
Contributor

still want to look into this further when I have cycles
/lifecycle frozen

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 3, 2021

@yselkowitz: The lifecycle/frozen label cannot be applied to Pull Requests.

In response to this:

still want to look into this further when I have cycles
/lifecycle frozen

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.

@yselkowitz
Copy link
Contributor

so much for following directions...
/remove-lifecycle stale

@openshift-ci openshift-ci bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 3, 2021
@openshift-bot
Copy link
Contributor

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci openshift-ci bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 1, 2021
@yselkowitz
Copy link
Contributor

/remove-lifecycle stale

@openshift-ci openshift-ci bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 1, 2021
@openshift-merge-robot
Copy link
Contributor

/bugzilla refresh

The requirements for Bugzilla bugs have changed, recalculating validity.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 6, 2021

@openshift-merge-robot: This pull request references Bugzilla bug 1838512, which is invalid:

  • expected the bug to be open, but it isn't
  • expected the bug to target the "4.10.0" release, but it targets "---" instead
  • expected the bug to be in one of the following states: NEW, ASSIGNED, ON_DEV, POST, POST, but it is CLOSED (WONTFIX) instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

Retaining the bugzilla/valid-bug label as it was manually added.

In response to this:

/bugzilla refresh

The requirements for Bugzilla bugs have changed, recalculating validity.

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.

@openshift-merge-robot
Copy link
Contributor

/bugzilla refresh

The requirements for Bugzilla bugs have changed, recalculating validity.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 6, 2021

@openshift-merge-robot: This pull request references Bugzilla bug 1838512, which is invalid:

  • expected the bug to be open, but it isn't
  • expected the bug to target the "4.10.0" release, but it targets "---" instead
  • expected the bug to be in one of the following states: NEW, ASSIGNED, ON_DEV, POST, POST, but it is CLOSED (WONTFIX) instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

Retaining the bugzilla/valid-bug label as it was manually added.

In response to this:

/bugzilla refresh

The requirements for Bugzilla bugs have changed, recalculating validity.

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.

@openshift-bot
Copy link
Contributor

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci openshift-ci bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 5, 2021
@ITD27M01
Copy link

ITD27M01 commented Dec 7, 2021

/remove-lifecycle stale

@openshift-ci openshift-ci bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 7, 2021
@openshift-bot
Copy link
Contributor

/bugzilla refresh

The requirements for Bugzilla bugs have changed (BZs linked to PRs on master branch need to target OCP 4.11), recalculating validity.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 28, 2022

@openshift-bot: This pull request references Bugzilla bug 1838512, which is invalid:

  • expected the bug to be open, but it isn't
  • expected the bug to target the "4.11.0" release, but it targets "---" instead
  • expected the bug to be in one of the following states: NEW, ASSIGNED, ON_DEV, POST, POST, but it is CLOSED (WONTFIX) instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

Retaining the bugzilla/valid-bug label as it was manually added.

In response to this:

/bugzilla refresh

The requirements for Bugzilla bugs have changed (BZs linked to PRs on master branch need to target OCP 4.11), recalculating validity.

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.

@wking
Copy link
Member

wking commented Jan 31, 2022

/bugzilla refresh

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 31, 2022

@wking: This pull request references Bugzilla bug 1838512, which is invalid:

  • expected the bug to be open, but it isn't
  • expected the bug to target the "4.11.0" release, but it targets "---" instead
  • expected the bug to be in one of the following states: NEW, ASSIGNED, ON_DEV, POST, POST, but it is CLOSED (WONTFIX) instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

Retaining the bugzilla/valid-bug label as it was manually added.

In response to this:

/bugzilla refresh

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.

@openshift-bot
Copy link
Contributor

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci openshift-ci bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 2, 2022
@openshift-bot
Copy link
Contributor

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten
/remove-lifecycle stale

@openshift-ci openshift-ci bot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jun 1, 2022
@openshift-bot
Copy link
Contributor

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

@openshift-ci openshift-ci bot closed this Jul 1, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 1, 2022

@openshift-bot: Closed this PR.

In response to this:

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

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.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 1, 2022

@damemi: An error was encountered removing this pull request from the external tracker bugs for bug 1838512 on the Bugzilla server at https://bugzilla.redhat.com. No known errors were detected, please see the full error message for details.

Full error message. response code 400 not 200

Please contact an administrator to resolve this issue, then request a bug refresh with /bugzilla refresh.

In response to this:

Bug 1838512: Set CGO_ENABLED=1 for cross compile on Mac

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants