Skip to content

Comments

Update k8s.io/test-infra dependency and fix GCP Mason#2447

Merged
istio-testing merged 1 commit intoistio:masterfrom
ixdy:update-gcp-mason
Mar 7, 2020
Merged

Update k8s.io/test-infra dependency and fix GCP Mason#2447
istio-testing merged 1 commit intoistio:masterfrom
ixdy:update-gcp-mason

Conversation

@ixdy
Copy link
Contributor

@ixdy ixdy commented Feb 29, 2020

Some recent changes to Boskos changed some of the CRDs slightly, which is now causing older Mason implementations to fail to register:

  error: "no kind "DRLCObjectList" is registered for version "boskos.k8s.io/v1" in scheme "pkg/runtime/scheme.go:101""   

Thankfully, the changes are fairly straightforward, and we can easily use a caching kubernetes client now, too. (The benefits of caching are probably fairly low, since DRLCs don't change that often.)

Relevant upstream changes for context:
kubernetes/test-infra#16206
kubernetes/test-infra#16249
kubernetes/test-infra#16367

@ixdy ixdy requested a review from a team as a code owner February 29, 2020 02:23
@istio-policy-bot
Copy link

😊 Welcome @ixdy! This is either your first contribution to the Istio test-infra repo, or it's been
awhile since you've been here.

You can learn more about the Istio working groups, code of conduct, and contributing guidelines
by referring to Contributing to Istio.

Thanks for contributing!

Courtesy of your friendly welcome wagon.

@googlebot googlebot added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label Feb 29, 2020
@istio-testing istio-testing added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Feb 29, 2020
@istio-testing
Copy link
Collaborator

Hi @ixdy. Thanks for your PR.

I'm waiting for a istio 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.

Details

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.

@clarketm
Copy link
Member

/ok-to-test

@istio-testing istio-testing added ok-to-test Set this label allow normal testing to take place for a PR not submitted by an Istio org member. and removed needs-ok-to-test labels Feb 29, 2020
@ixdy ixdy force-pushed the update-gcp-mason branch from 89478f8 to 1c1268c Compare March 5, 2020 03:40
@ixdy
Copy link
Contributor Author

ixdy commented Mar 5, 2020

I additionally needed to update some unrelated files, since some Prow internal data structures have changed.

@clarketm
Copy link
Member

clarketm commented Mar 5, 2020

I additionally needed to update some unrelated files, since some Prow internal data structures have changed.

Thanks! Additionally, can you run make gen then commit the changes.

@istio-testing istio-testing added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 5, 2020
@ixdy
Copy link
Contributor Author

ixdy commented Mar 5, 2020

it looks like it's unhappy about licenses, and I have no idea how to fix this:

ERROR: Some modules have unrecognized licenses:
  github.com/DataDog/zstd: similar to NCSA, 63.67% confidence, path '/home/prow/go/pkg/mod/github.com/!data!dog/zstd@v1.4.1/LICENSE'
  github.com/emirpasic/gods: path '/home/prow/go/pkg/mod/github.com/emirpasic/gods@v1.12.0/LICENSE'
  github.com/kevinburke/ssh_config: path '/home/prow/go/pkg/mod/github.com/kevinburke/ssh_config@v0.0.0-20190725054713-01f96b0aa0cd/LICENSE'
  github.com/tektoncd/pipeline: path '/home/prow/go/pkg/mod/github.com/tektoncd/pipeline@v0.10.1/third_party/github.com/ghodss/yaml/LICENSE'
  github.com/tektoncd/pipeline: similar to BSD-3-Clause-Clear, 89.66% confidence, path '/home/prow/go/pkg/mod/github.com/tektoncd/pipeline@v0.10.1/third_party/github.com/gogo/protobuf/LICENSE'
  github.com/tektoncd/pipeline: path '/home/prow/go/pkg/mod/github.com/tektoncd/pipeline@v0.10.1/third_party/github.com/jmespath/go-jmespath/LICENSE'
  github.com/tektoncd/pipeline: path '/home/prow/go/pkg/mod/github.com/tektoncd/pipeline@v0.10.1/third_party/sigs.k8s.io/yaml/LICENSE'
ERROR: Some modules have no discernible license:
  github.com/otiai10/curr
  github.com/pelletier/go-buffruneio
common/Makefile.common.mk:64: recipe for target 'lint-licenses' failed

@clarketm
Copy link
Member

clarketm commented Mar 5, 2020

it looks like it's unhappy about licenses, and I have no idea how to fix this:

ERROR: Some modules have unrecognized licenses:
  github.com/DataDog/zstd: similar to NCSA, 63.67% confidence, path '/home/prow/go/pkg/mod/github.com/!data!dog/zstd@v1.4.1/LICENSE'
  github.com/emirpasic/gods: path '/home/prow/go/pkg/mod/github.com/emirpasic/gods@v1.12.0/LICENSE'
  github.com/kevinburke/ssh_config: path '/home/prow/go/pkg/mod/github.com/kevinburke/ssh_config@v0.0.0-20190725054713-01f96b0aa0cd/LICENSE'
  github.com/tektoncd/pipeline: path '/home/prow/go/pkg/mod/github.com/tektoncd/pipeline@v0.10.1/third_party/github.com/ghodss/yaml/LICENSE'
  github.com/tektoncd/pipeline: similar to BSD-3-Clause-Clear, 89.66% confidence, path '/home/prow/go/pkg/mod/github.com/tektoncd/pipeline@v0.10.1/third_party/github.com/gogo/protobuf/LICENSE'
  github.com/tektoncd/pipeline: path '/home/prow/go/pkg/mod/github.com/tektoncd/pipeline@v0.10.1/third_party/github.com/jmespath/go-jmespath/LICENSE'
  github.com/tektoncd/pipeline: path '/home/prow/go/pkg/mod/github.com/tektoncd/pipeline@v0.10.1/third_party/sigs.k8s.io/yaml/LICENSE'
ERROR: Some modules have no discernible license:
  github.com/otiai10/curr
  github.com/pelletier/go-buffruneio
common/Makefile.common.mk:64: recipe for target 'lint-licenses' failed

I am not entirely certain either. It is either a bug with the license resolution logic or an issue with the dependencies. Related discussion: istio/istio#20379. @howardjohn or @sdake any idea how to resolve?

@ixdy ixdy force-pushed the update-gcp-mason branch from 4747f13 to 05a745b Compare March 6, 2020 01:29
@ixdy
Copy link
Contributor Author

ixdy commented Mar 6, 2020

ok, I think I've fixed some of these issues by pinning to newer versions (which include the LICENSE files), though a bunch are still failing; I suspect they'll need to be whitelisted:

ERROR: Some modules have unrecognized licenses:
  github.com/DataDog/zstd: similar to NCSA, 63.67% confidence, path '/go/pkg/mod/github.com/!data!dog/zstd@v1.4.1/LICENSE'
  github.com/emirpasic/gods: path '/go/pkg/mod/github.com/emirpasic/gods@v1.12.0/LICENSE'
  github.com/kevinburke/ssh_config: path '/go/pkg/mod/github.com/kevinburke/ssh_config@v0.0.0-20190725054713-01f96b0aa0cd/LICENSE'
  github.com/otiai10/curr: path '/go/pkg/mod/github.com/otiai10/curr@v0.0.0-20190513014714-f5a3d24e5776/LICENSE'
  github.com/tektoncd/pipeline: path '/go/pkg/mod/github.com/tektoncd/pipeline@v0.10.1/third_party/github.com/ghodss/yaml/LICENSE'
  github.com/tektoncd/pipeline: similar to BSD-3-Clause-Clear, 89.66% confidence, path '/go/pkg/mod/github.com/tektoncd/pipeline@v0.10.1/third_party/github.com/gogo/protobuf/LICENSE'
  github.com/tektoncd/pipeline: path '/go/pkg/mod/github.com/tektoncd/pipeline@v0.10.1/third_party/github.com/jmespath/go-jmespath/LICENSE'
  github.com/tektoncd/pipeline: path '/go/pkg/mod/github.com/tektoncd/pipeline@v0.10.1/third_party/sigs.k8s.io/yaml/LICENSE'

Going one-by-one:

@ixdy
Copy link
Contributor Author

ixdy commented Mar 6, 2020

All of the third-party dependencies under tektoncd/pipeline are already whitelisted, just not with their third-party paths. I'm not sure whether the tool should be taught to recognize this, or if we should whitelist them explicitly.

Any guidance on what I should do about the other dependencies?

@clarketm
Copy link
Member

clarketm commented Mar 7, 2020

All of the third-party dependencies under tektoncd/pipeline are already whitelisted, just not with their third-party paths. I'm not sure whether the tool should be taught to recognize this, or if we should whitelist them explicitly.

Any guidance on what I should do about the other dependencies?

Whitelisting those specific modules should be fine.

@howardjohn
Copy link
Member

howardjohn commented Mar 7, 2020 via email

@ixdy
Copy link
Contributor Author

ixdy commented Mar 7, 2020

I'm trying to understand where this dependency is coming from, but I'm failing.

Nothing in k8s.io/test-infra directly depends on it. If I try downloading the deps into vendor/, nothing there depends on it. go mod why isn't helping either.

@clarketm
Copy link
Member

clarketm commented Mar 7, 2020

I have not traced it all the way but it is from github.com/tektoncd/pipeline v0.10.1

@howardjohn
Copy link
Member

howardjohn commented Mar 7, 2020

Ahh thats right... we have had this problem before. K8s license checker checks code it uses from vendor, which is pruned, whereas istio's is checking all transitive dependencies by go mod graph.

github.com/tektoncd/plumbing > github.com/google/go-licenses > github.com/otiai10/copy > github.com/otiai10/mint > github.com/otiai10/curr

I don't know what we do now.

@ixdy
Copy link
Contributor Author

ixdy commented Mar 7, 2020

I opened otiai10/curr#2. We'll see if the maintainer is willing to switch the license.

@clarketm clarketm added the do-not-merge/hold Block automatic merging of a PR. label Mar 7, 2020
@ixdy
Copy link
Contributor Author

ixdy commented Mar 7, 2020

oh, this is going to need me to re-run make gen, right.

Additionally, update (and pin)
- github.com/otiai10/curr
- github.com/otiai10/mint
- github.com/pelletier/go-buffruneio
@ixdy ixdy force-pushed the update-gcp-mason branch from 7ba40df to f82e961 Compare March 7, 2020 18:37
@ixdy
Copy link
Contributor Author

ixdy commented Mar 7, 2020

ok, looks like this is hopefully set once istio/common-files#206 gets in.

@howardjohn
Copy link
Member

thanks! sorry for the license issues. the common files update should automatically get merged in here in a bit

@howardjohn
Copy link
Member

howardjohn commented Mar 7, 2020 via email

@clarketm clarketm removed the do-not-merge/hold Block automatic merging of a PR. label Mar 7, 2020
@istio-testing istio-testing merged commit c06282b into istio:master Mar 7, 2020
@ixdy
Copy link
Contributor Author

ixdy commented Mar 7, 2020

ok, the last remaining question: does anything automatically build and publish gcr.io/istio-testing/mason? or is that a manual process?

@clarketm
Copy link
Member

clarketm commented Mar 7, 2020

ok, the last remaining question: does anything automatically build and publish gcr.io/istio-testing/mason? or is that a manual process?

It is currently manual. make -C boskos mason-image should do the trick. I can automate this for the next time (#2484).

@ixdy
Copy link
Contributor Author

ixdy commented Mar 7, 2020

can you build and push when convenient? (doesn't need to happen this weekend.) I don't have access to the istio-testing project.

@clarketm
Copy link
Member

clarketm commented Mar 7, 2020

can you build and push when convenient? (doesn't need to happen this weekend.) I don't have access to the istio-testing project.

gcr.io/istio-testing/mason:v20200307-1.5.0-19-gc06282b0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. ok-to-test Set this label allow normal testing to take place for a PR not submitted by an Istio org member. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants