Overview
Request 640141 superseded
- Add patch to lib/container_server.go fixing go1.11 compatibility.
* Tested with golang(API) == 1.10 and golang(API) == 1.11, OK
* Upstream git master commit
https://github.com/kubernetes-sigs/cri-o/commit/0bd30872028b5ed2d0eb7febb39f034b5f2da72a
contains 1 hunk adding missing argument in format string of
calls to:
# github.com/kubernetes-incubator/cri-o/lib
lib/container_server.go:309: Debugf call needs 1 arg but has 2 args
lib/container_server.go:317: Debugf call needs 1 arg but has 2 args
...
FAIL github.com/kubernetes-incubator/cri-o/lib [build failed]
Calls in question:
logrus.Debugf("loaded new pod sandbox %s", sandboxID, err)
logrus.Debugf("loaded new pod container %s", containerID, err)
require another argument to the string format (": %v" per upstream):
logrus.Debugf("loaded new pod sandbox %s: %v", sandboxID, err)
logrus.Debugf("loaded new pod container %s: %v", containerID, err)
Patch contents not available in upstream cri-o released versions:
cri-o-1.11.3
cri-o-1.11.4
cri-o-1.11.5
cri-o-1.11.6
Filed upstream issue requesting patch contents in released version:
https://github.com/kubernetes-sigs/cri-o/issues/1827
- Created by jfkw
- In state superseded
- Package maintainer: jfkw
- Superseded by 640653
Request History
jfkw created request
- Add patch to lib/container_server.go fixing go1.11 compatibility.
* Tested with golang(API) == 1.10 and golang(API) == 1.11, OK
* Upstream git master commit
https://github.com/kubernetes-sigs/cri-o/commit/0bd30872028b5ed2d0eb7febb39f034b5f2da72a
contains 1 hunk adding missing argument in format string of
calls to:
# github.com/kubernetes-incubator/cri-o/lib
lib/container_server.go:309: Debugf call needs 1 arg but has 2 args
lib/container_server.go:317: Debugf call needs 1 arg but has 2 args
...
FAIL github.com/kubernetes-incubator/cri-o/lib [build failed]
Calls in question:
logrus.Debugf("loaded new pod sandbox %s", sandboxID, err)
logrus.Debugf("loaded new pod container %s", containerID, err)
require another argument to the string format (": %v" per upstream):
logrus.Debugf("loaded new pod sandbox %s: %v", sandboxID, err)
logrus.Debugf("loaded new pod container %s: %v", containerID, err)
Patch contents not available in upstream cri-o released versions:
cri-o-1.11.3
cri-o-1.11.4
cri-o-1.11.5
cri-o-1.11.6
Filed upstream issue requesting patch contents in released version:
https://github.com/kubernetes-sigs/cri-o/issues/1827
Hi Jeff, thanks a lot for taking the time to open the submit request!
Unfortunately, I don't think that downstream patching our package is necessary as the patch fixes a build error with go 1.11 that we currently don't ship. Instead, I'd prefer to pin our build to 1.10 which we should do in any case as that's the version upstream is tested with. As the patch will be merged into the release branches, it will be available with the version update to our package.
If we go with the downstream patch, we need to open a bug in Bugzilla and do the entire dance that I'd like to avoid.
Jeff, would you open another PR pinning the go version to 1.10?
Certainly. Is the preferred method of pinning go version:
BuildRequires: golang(API) == 1.10
Um but if you do that, then we'll have to reject https://build.opensuse.org/request/show/640330
As it looks like, pinning it to 1.10 wouldn't work with go bump to 1.11 as we're not shipping multiple versions of go.
Looks like, we have to go with this SR. Thanks @jfkw and @RBrownSUSE!
LGTM/ACK from my side on this SR.
Hi, after talking with Valentin and considering as TW only has one go version at a time, I'm okay with accepting the patch.
But I'd prefer a .patch filename that explains more about why we have the patch rather than technical details about what's in it
go-1.11-compat-backport.patch would have been the name I'd pick for it..so when cri-o 1.12 comes around I won't forget that we can drop this right away
Can you supersede this SR with a such a change?