Bump x/sys and opencontainers/runc to support Risc-V architecture #82349

carlosedp · 2019-09-04T22:01:59Z

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

Bump modules to allow building Kubernetes on Risc-V architecture.
Risc-V software support tracker on https://github.com/carlosedp/riscv-bringup

Special notes for your reviewer:

This PR depends on #82342 so it might need a rebase after that one get's merged.

Does this PR introduce a user-facing change?:

NONE

Signed-off-by: Carlos de Paula <me@carlosedp.com>

k8s-ci-robot · 2019-09-04T22:02:08Z

Hi @carlosedp. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

BenTheElder · 2019-09-04T22:24:58Z

/ok-to-test

BenTheElder

x/sys should be fine, runc has some possibly noteworthy changes

BenTheElder · 2019-09-04T22:28:07Z

vendor/github.com/opencontainers/runc/libcontainer/cgroups/systemd/apply_systemd.go

 func UseSystemd() bool {
-	if !systemdUtil.IsRunningSystemd() {
+	if !isRunningSystemd() {


https://github.com/coreos/go-systemd/blob/fd7a80b32e1fc73e890fde45604ed5009dc817a3/util/util.go#L72

confirming this is the same method, just lifted into this package (no behavior change)

BenTheElder · 2019-09-04T22:30:44Z

vendor/github.com/opencontainers/runc/libcontainer/cgroups/systemd/apply_systemd.go

@@ -208,10 +160,6 @@ func (m *Manager) Apply(pid int) error {

 	// if we create a slice, the parent is defined via a Wants=
 	if strings.HasSuffix(unitName, ".slice") {
-		// This was broken until systemd v229, but has been back-ported on RHEL environments >= 219


possibly notable change opencontainers/runc#2117

Transient units (and transient slice units) have been available for quite a long time and RHEL 7 with systemd v219 (likely the oldest OS we care about at this point) supports that. A system running a systemd without these features is likely to break a lot of other stuff that runc/libcontainer care about.

BenTheElder · 2019-09-04T22:30:56Z

vendor/github.com/opencontainers/runc/libcontainer/cgroups/systemd/apply_systemd.go

-		if hasDelegateSlice {
-			// systemd 237 and above no longer allows delegation on a slice
-			properties = append(properties, newProp("Delegate", true))
-		}


possibly notable change opencontainers/runc#2117

Regarding delegated slices, modern systemd doesn't allow it and runc/libcontainer run fine on it, so we might as well just stop requesting it on older versions of systemd which allowed it. (Those versions never really changed behavior significantly when that option was passed anyways.)

carlosedp · 2019-09-04T23:15:07Z

Any idea on why the failing tests @BenTheElder ?

BenTheElder · 2019-09-04T23:16:54Z

go: finding github.com/urfave/cli v1.21.0
go: finding golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2
Skipping golang.org/x/... dependencies, pass --all to include

Use the given commands to remove pinned module versions that aren't actually used:
GO111MODULE=on go mod edit -dropreplace github.com/urfave/cli

https://prow.k8s.io/view/gcs/kubernetes-jenkins/pr-logs/pull/82349/pull-kubernetes-dependencies/1169375978296709121

carlosedp · 2019-09-05T18:48:32Z

/retest

carlosedp · 2019-09-05T19:19:31Z

Now it's GTG @BenTheElder :) Let's ping the approvers.

fedebongio · 2019-09-05T20:21:29Z

/cc @liggitt

BenTheElder · 2019-09-05T21:41:41Z

I spoke to the upstream author re opencontainers/runc#2117 and in theory no supported distros should have systemd old enough for this to be a problem.

Still might be release note worthy.
/lgtm
/approve

k8s-ci-robot · 2019-09-05T21:42:14Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: BenTheElder, carlosedp
To complete the pull request process, please assign lavalamp
You can assign the PR to them by writing /assign @lavalamp 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:

OWNERS
staging/src/k8s.io/api/OWNERS

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

carlosedp · 2019-09-06T21:24:51Z

/assign @lavalamp

carlosedp · 2019-09-10T21:39:19Z

/kind cleanup

liggitt · 2019-09-12T13:26:27Z

go.mod

@@ -422,7 +422,7 @@ replace (
 	golang.org/x/net => golang.org/x/net v0.0.0-20190812203447-cdfb69ac37fc
 	golang.org/x/oauth2 => golang.org/x/oauth2 v0.0.0-20190402181905-9f3314589c9a
 	golang.org/x/sync => golang.org/x/sync v0.0.0-20181108010431-42b317875d0f
-	golang.org/x/sys => golang.org/x/sys v0.0.0-20190209173611-3b5209105503


we were intentionally pinned to 3b52091 because it is the branch matching our current version of go (https://github.com/golang/sys/commits/release-branch.go1.12). I would expect us to stay on that revision until we move to go 1.13, and then move to the https://github.com/golang/sys/commits/release-branch.go1.13 branch unless there's a compelling reason to move away from the go team's maintained release branches.

Commit 3b52091 still doesn't have support for Risc-V. The earliest commit that has the support and fixes is golang/sys@9eafafc. I checked that x/sys release branch 1.13 already has this.
What's the plans on moving to Go 1.13?

What's the plans on moving to Go 1.13?

Tracked in #82531, planned for 1.17, but not as trivial a move as previous go version bumps

go1.13 support landed in #82809

liggitt · 2019-09-12T13:29:30Z

rebasing on master now that #82342 has merged will shrink the size of this

cc @kubernetes/sig-node-pr-reviews for runc bump approval

unless there's a compelling reason not to, I'd expect us to stay on the versions of golang.org/x/... dependencies that match our go version (#82349 (comment))

carlosedp · 2019-11-26T22:38:51Z

Folks, let me know whenever I should rebase this to be merged.
Thanks!

liggitt · 2019-11-26T22:48:08Z

Rebasing on master should shrink the size of this. I still would not expect to run ahead of our go release for golang.org/x/sys unless there was a strong reason to do so. Is there a discussion around prioritization of adding support for this architecture that I'm missing?

carlosedp · 2019-11-26T22:57:44Z

No priorization on the arch. I've been building the packages locally and applying changes. I'll demo K8s on Risc-V arch on the Risc-V Summit now in december so I came to check the status.

K8s 1.17 will be built with 1.13 already?

carlosedp · 2019-11-26T23:31:10Z

@liggitt I've just checked the commits below and they have already bumped up the dependencies on master. I believe 1.18 will be cut from this, right?

660b17d
708a917

With this, I believe this PR can be closed.

liggitt · 2019-11-27T00:39:21Z

1.17 will be cut with go1.13
1.18 will be cut from what is in master

carlosedp · 2019-11-29T13:51:03Z

Hi @liggitt , I've just built K8s with latest master and it worked fine. All libraries have been bumped. I need to update hack/lib/golang.sh adding riscv64 to the list of architectures so make all KUBE_BUILD_PLATFORMS=linux/riscv64 works fine. I'll submit another PR and close this, ok?

liggitt · 2019-11-29T17:09:10Z

I'm unsure of all the considerations for supporting a new architecture… does that mean we claim test/support for it?

dims · 2019-11-30T02:44:00Z

@liggitt we already support builds for ppc64le, s390x etc and we don't claim "support" for those arches. so this would be similar to those i think

carlosedp · 2019-11-30T22:33:16Z

Exactly, It's not claiming support or officially providing images right now but the ability to correctly build the packages using official pipeline and scripts.

liggitt · 2019-12-02T16:03:12Z

Exactly, It's not claiming support or officially providing images right now but the ability to correctly build the packages using official pipeline and scripts.

Ok. I'll defer to the owners of those build/release scripts for approval on the follow up PR.

BenTheElder · 2019-12-02T19:56:16Z

Hi @liggitt , I've just built K8s with latest master and it worked fine. All libraries have been bumped. I need to update hack/lib/golang.sh adding riscv64 to the list of architectures so make all KUBE_BUILD_PLATFORMS=linux/riscv64 works fine. I'll submit another PR and close this, ok?

👍, happy to review this

carlosedp · 2019-12-06T19:24:17Z

Latest master branch builds correctly on riscv64.

@BenTheElder I've opened #86011 to address the build script.

Closing this.

Bump gonvml module and remove CGO dependency.

8cd98fb

Signed-off-by: Carlos de Paula <me@carlosedp.com>

k8s-ci-robot requested review from adohe-zz, andrewsykim and a team September 4, 2019 22:02

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Sep 4, 2019

BenTheElder reviewed Sep 4, 2019

View reviewed changes

k8s-ci-robot requested a review from liggitt September 5, 2019 20:21

k8s-ci-robot assigned BenTheElder Sep 5, 2019

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 5, 2019

k8s-ci-robot assigned lavalamp Sep 6, 2019

k8s-ci-robot added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. and removed needs-kind Indicates a PR lacks a `kind/foo` label and requires one. labels Sep 10, 2019

liggitt reviewed Sep 12, 2019

View reviewed changes

carlosedp mentioned this pull request Nov 26, 2019

unix: fix epoll_event padding on riscv64 golang/sys#40

Closed

dims mentioned this pull request Dec 4, 2019

Conformance results for v1.16/inspur-iop-mips64el cncf/k8s-conformance#779

Closed

carlosedp closed this Dec 6, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bump x/sys and opencontainers/runc to support Risc-V architecture #82349

Bump x/sys and opencontainers/runc to support Risc-V architecture #82349

carlosedp commented Sep 4, 2019 •

edited

Loading

k8s-ci-robot commented Sep 4, 2019

BenTheElder commented Sep 4, 2019

BenTheElder left a comment

BenTheElder Sep 4, 2019

BenTheElder Sep 4, 2019

BenTheElder Sep 4, 2019

carlosedp commented Sep 4, 2019

BenTheElder commented Sep 4, 2019 •

edited

Loading

carlosedp commented Sep 5, 2019

carlosedp commented Sep 5, 2019

fedebongio commented Sep 5, 2019

BenTheElder commented Sep 5, 2019

k8s-ci-robot commented Sep 5, 2019

carlosedp commented Sep 6, 2019

carlosedp commented Sep 10, 2019

liggitt Sep 12, 2019

carlosedp Sep 12, 2019

liggitt Sep 12, 2019

liggitt Nov 26, 2019

liggitt commented Sep 12, 2019

carlosedp commented Nov 26, 2019

liggitt commented Nov 26, 2019

carlosedp commented Nov 26, 2019 •

edited

Loading

carlosedp commented Nov 26, 2019

liggitt commented Nov 27, 2019

carlosedp commented Nov 29, 2019

liggitt commented Nov 29, 2019

dims commented Nov 30, 2019

carlosedp commented Nov 30, 2019

liggitt commented Dec 2, 2019

BenTheElder commented Dec 2, 2019

carlosedp commented Dec 6, 2019

Bump x/sys and opencontainers/runc to support Risc-V architecture #82349

Bump x/sys and opencontainers/runc to support Risc-V architecture #82349

Conversation

carlosedp commented Sep 4, 2019 • edited Loading

k8s-ci-robot commented Sep 4, 2019

BenTheElder commented Sep 4, 2019

BenTheElder left a comment

Choose a reason for hiding this comment

BenTheElder Sep 4, 2019

Choose a reason for hiding this comment

BenTheElder Sep 4, 2019

Choose a reason for hiding this comment

BenTheElder Sep 4, 2019

Choose a reason for hiding this comment

carlosedp commented Sep 4, 2019

BenTheElder commented Sep 4, 2019 • edited Loading

carlosedp commented Sep 5, 2019

carlosedp commented Sep 5, 2019

fedebongio commented Sep 5, 2019

BenTheElder commented Sep 5, 2019

k8s-ci-robot commented Sep 5, 2019

carlosedp commented Sep 6, 2019

carlosedp commented Sep 10, 2019

liggitt Sep 12, 2019

Choose a reason for hiding this comment

carlosedp Sep 12, 2019

Choose a reason for hiding this comment

liggitt Sep 12, 2019

Choose a reason for hiding this comment

liggitt Nov 26, 2019

Choose a reason for hiding this comment

liggitt commented Sep 12, 2019

carlosedp commented Nov 26, 2019

liggitt commented Nov 26, 2019

carlosedp commented Nov 26, 2019 • edited Loading

carlosedp commented Nov 26, 2019

liggitt commented Nov 27, 2019

carlosedp commented Nov 29, 2019

liggitt commented Nov 29, 2019

dims commented Nov 30, 2019

carlosedp commented Nov 30, 2019

liggitt commented Dec 2, 2019

BenTheElder commented Dec 2, 2019

carlosedp commented Dec 6, 2019

carlosedp commented Sep 4, 2019 •

edited

Loading

BenTheElder commented Sep 4, 2019 •

edited

Loading

carlosedp commented Nov 26, 2019 •

edited

Loading