Bring vendor dir in sync #523
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
size/XXL
| @@ -1,4 +1,4 @@ | |||
| #!/usr/bin/env bash -e | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
Is this the right way to add a new step to CI?
There was a problem hiding this comment.
You have to configure prow to run some sort of command. Right now it runs the three scripts for CI as three different tests. We can either augment an existing script or add a new script (and configure a new presumbit).
There was a problem hiding this comment.
I don't really have an opinion on augment existing script vs add a new script + configure a new presubmit - Any preference from your side?
There was a problem hiding this comment.
+1 to a new script + configure a new presubmit
There was a problem hiding this comment.
Fine with me. Can you tell me where a new presubmit gets configured?
|
/assign @roberthbailey |
alvaroaleman
force-pushed
the
fix-dep-ensure
branch
from
a82a3a7 to
12e0c77
Compare
Gopkg.toml
Outdated
| @@ -22,11 +22,11 @@ required = [ | |||
|
|
|||
| [[constraint]] | |||
| name="sigs.k8s.io/controller-runtime" | |||
| version="v0.1.1" | |||
| version="v0.1.3" | |||
There was a problem hiding this comment.
This constraint was plain wrong, dep does find a solution but that is not buildable because:
# sigs.k8s.io/cluster-api/pkg/controller/cluster [sigs.k8s.io/cluster-api/pkg/controller/cluster.test]
pkg/controller/cluster/controller.go:129:43: unknown field 'RequeueAfter' in struct literal of type reconcile.Result
# sigs.k8s.io/cluster-api/pkg/controller/machine [sigs.k8s.io/cluster-api/pkg/controller/machine.test]
pkg/controller/machine/controller.go:168:44: unknown field 'RequeueAfter' in struct literal of type reconcile.Result
# sigs.k8s.io/cluster-api/cmd/clusterctl/cmd
cmd/clusterctl/cmd/validate_cluster.go:72:91: mgr.GetRESTMapper undefined (type manager.Manager has no field or method GetRESTMapper)
# sigs.k8s.io/cluster-api/cmd/clusterctl/validation [sigs.k8s.io/cluster-api/cmd/clusterctl/validation.test]
cmd/clusterctl/validation/validate_cluster_api_objects_test.go:424:79: mgr.GetRESTMapper undefined (type manager.Manager has no field or method GetRESTMapper)
# sigs.k8s.io/cluster-api/cmd/clusterctl/cmd [sigs.k8s.io/cluster-api/cmd/clusterctl/cmd.test]
cmd/clusterctl/cmd/validate_cluster.go:72:91: mgr.GetRESTMapper undefined (type manager.Manager has no field or method GetRESTMapper)
The first version of controller-runtime that does have a field RequeueAfter in the reconcile.Result struct is v0.1.3: kubernetes-sigs/controller-runtime@83f4800
There was a problem hiding this comment.
Why is this constraint wrong? It's in the section with STANZAS BELOW ARE GENERATED AND MAY BE WRITTEN - DO NOT MODIFY BELOW THIS LINE. which is auto-generated by kubebuilder.
There was a problem hiding this comment.
This is an upstream issue, the versions in the generated Gopkg.toml are hardcoded: kubernetes-sigs/kubebuilder/issues/419
However, we can just remove the # STANZAS BELOW ARE GENERATED AND MAY BE WRITTEN - DO NOT MODIFY BELOW THIS LINE. line from the Gopkg.toml at least until the upstream issue gets resolved. When doing that, running kubebuilder update results in a
$ kubebuilder update
2018/10/07 11:16:50 error updating vendor dependecies skipping modifying Gopkg.toml - file already exists and is unmanaged
Any objections to that?
There was a problem hiding this comment.
Explained the reasoning why the version says v0.1.1 in the Stanza in the issue here kubernetes-sigs/kubebuilder#419 (comment)
In order to bring deps in sync, dep ensure -update sigs.k8s.io/controller-runtime sigs.k8s.io/controller-tools should be enough to fetch latest version.
alvaroaleman
changed the title
alvaroaleman
changed the title
|
cc @xmudrii |
|
/ok-to-test |
k8s-ci-robot
removed
the
needs-ok-to-test
|
/retest The tests here also fail due to a |
|
/retest I've created a PR to get these timeouts configurable: kubernetes-sigs/controller-runtime#169 |
alvaroaleman
force-pushed
the
fix-dep-ensure
branch
from
12e0c77 to
8eb9308
Compare
alvaroaleman
changed the title
|
@roberthbailey I've removed the change of the |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alvaroaleman, roberthbailey 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 |
k8s-ci-robot
added
the
approved
|
/retest |
relative link for doc
What this PR does / why we need it:
Brings the checked in vendor in sync
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:
Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.
Release note: