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

hack/update: base library #9526

Closed
wants to merge 0 commits into from

Conversation

prezha
Copy link
Contributor

@prezha prezha commented Oct 22, 2020

fixes: #9464

pr #9464 ("automate update processes") split: part 1/4 - this one should be the first one to merge

includes:

  • update.go: package init, base struct w/ method, and functions
  • filesystem.go: update local filesystem repo
  • github.go: get greatest stable and latest releases, find/create PR,
    and update remote GitHub repo
  • registry.go: manage images (eg, pull, tag, etc.) and update container
    registries

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Oct 22, 2020
@k8s-ci-robot
Copy link
Contributor

Hi @prezha. 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.

@k8s-ci-robot k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Oct 22, 2020
@minikube-bot
Copy link
Collaborator

Can one of the admins verify this patch?

Copy link
Member

@medyagh medyagh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you for splitting the work into smaller parts
do you mind choosing a more declaring name, for both the PR and also the folder, so it is obvious what is the "update" for

@prezha
Copy link
Contributor Author

prezha commented Oct 22, 2020

@medyagh no worries
also, please note that this part is still wip - i'll update when it's ready (in the next few hours) so that it's not reviewed multiple times... this part is now ready for review

not sure how to rename the update folder (and the pr), as it will contain more declaring subfolders and files like:

  • part 2 pr would be named hack-update--kubernetes-version and will contain script: hack/update/kubernetes_version/update_kubernetes_version.go,
  • part 3 pr would be named hack-update--golang-version and will contain script: hack/update/golang_version/update_golang_version.go, and
  • part 4 pr would be named hack-update--kicbase-version and will contain script: hack/update/kicbase_version/update_kicbase_version.go

btw, update is also the name of the package, so we have eg, update.Item, update.Apply, update.TagImage etc. - that was a line of thought

does that makes sense or should we have more specific names? (if latter - please help with proposals! :))

edit: i'll proceed with other parts and will happily change the names into something more appropriate if needed

@prezha prezha requested a review from medyagh October 23, 2020 00:18
@prezha prezha force-pushed the hack-update--base-libs branch from 474b81c to 296f7d4 Compare October 28, 2020 19:52
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Oct 28, 2020
@prezha
Copy link
Contributor Author

prezha commented Oct 28, 2020

this one should be ok to merge now (lint error hack/update/kubernetes_version/update_kubernetes_version.go:76:32: GHVersions not declared by package update is covered in pr #9529)

@TravisBuddy
Copy link

Travis tests have failed

Hey @prezha,
Please read the following log in order to understand the failure reason.
It'll be awesome if you fix what's wrong and commit the changes.

1st Build

View build log

make test
PATH="/home/travis/gopath/bin:/home/travis/.gimme/versions/go1.15.2.linux.amd64/bin:/home/travis/bin:/home/travis/bin:/home/travis/.local/bin:/usr/local/lib/jvm/openjdk11/bin:/opt/pyenv/shims:/home/travis/.phpenv/shims:/home/travis/perl5/perlbrew/bin:/home/travis/.nvm/versions/node/v8.12.0/bin:/home/travis/.rvm/gems/ruby-2.5.3/bin:/home/travis/.rvm/gems/ruby-2.5.3@global/bin:/home/travis/.rvm/rubies/ruby-2.5.3/bin:/home/travis/gopath/bin:/home/travis/.gimme/versions/go1.11.1.linux.amd64/bin:/usr/local/maven-3.6.3/bin:/usr/local/cmake-3.12.4/bin:/usr/local/clang-7.0.0/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/snap/bin:/home/travis/.rvm/bin:/home/travis/.phpenv/bin:/opt/pyenv/bin:/home/travis/.yarn/bin:/home/travis/gopath/bin" go-bindata -nomemcopy -o pkg/minikube/assets/assets.go -pkg assets deploy/addons/...
gofmt -s -w pkg/minikube/assets/assets.go
PATH="/home/travis/gopath/bin:/home/travis/.gimme/versions/go1.15.2.linux.amd64/bin:/home/travis/bin:/home/travis/bin:/home/travis/.local/bin:/usr/local/lib/jvm/openjdk11/bin:/opt/pyenv/shims:/home/travis/.phpenv/shims:/home/travis/perl5/perlbrew/bin:/home/travis/.nvm/versions/node/v8.12.0/bin:/home/travis/.rvm/gems/ruby-2.5.3/bin:/home/travis/.rvm/gems/ruby-2.5.3@global/bin:/home/travis/.rvm/rubies/ruby-2.5.3/bin:/home/travis/gopath/bin:/home/travis/.gimme/versions/go1.11.1.linux.amd64/bin:/usr/local/maven-3.6.3/bin:/usr/local/cmake-3.12.4/bin:/usr/local/clang-7.0.0/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/snap/bin:/home/travis/.rvm/bin:/home/travis/.phpenv/bin:/opt/pyenv/bin:/home/travis/.yarn/bin:/home/travis/gopath/bin" go-bindata -nomemcopy -o pkg/minikube/translate/translations.go -pkg translate translations/...
gofmt -s -w pkg/minikube/translate/translations.go
MINIKUBE_LDFLAGS="-X k8s.io/minikube/pkg/version.version=v1.14.2 -X k8s.io/minikube/pkg/version.isoVersion=v1.14.0 -X k8s.io/minikube/pkg/version.isoPath=minikube/iso -X k8s.io/minikube/pkg/version.gitCommitID="85b916981a29ee40585eab0a4a91de5b8e3d245a" -X k8s.io/minikube/pkg/version.storageProvisionerVersion=v3" ./test.sh
= make lint =============================================================
golangci/golangci-lint info checking GitHub for tag 'v1.30.0'
golangci/golangci-lint info found version: 1.30.0 for v1.30.0/linux/amd64
golangci/golangci-lint info installed out/linters/golangci-lint
WARN [runner] Can't run linter goanalysis_metalinter: S1016: failed prerequisites: [([email protected]/minikube/hack/update/kubernetes_version, [email protected]/minikube/hack/update/kubernetes_version): analysis skipped: errors in package: [/home/travis/gopath/src/github.com/kubernetes/minikube/hack/update/kubernetes_version/update_kubernetes_version.go:76:32: GHVersions not declared by package update]] 
WARN [runner] Can't run linter unused: buildir: analysis skipped: errors in package: [/home/travis/gopath/src/github.com/kubernetes/minikube/hack/update/kubernetes_version/update_kubernetes_version.go:76:32: GHVersions not declared by package update] 
ERRO Running error: buildir: analysis skipped: errors in package: [/home/travis/gopath/src/github.com/kubernetes/minikube/hack/update/kubernetes_version/update_kubernetes_version.go:76:32: GHVersions not declared by package update] 
Makefile:434: recipe for target 'lint-ci' failed
make[1]: *** [lint-ci] Error 3
= go mod ================================================================
ok
= boilerplate ===========================================================
ok
Makefile:315: recipe for target 'test' failed
make: *** [test] Error 4
travis_time:end:108b7ff0:start=1603935856795869053,finish=1603936072378825257,duration=215582956204,event=script
TravisBuddy Request Identifier: 8dd81090-198e-11eb-a6ec-b9eb403ed3be

@medyagh
Copy link
Member

medyagh commented Oct 30, 2020

PR needs lint

Copy link
Member

@medyagh medyagh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

plz check lint

@prezha
Copy link
Contributor Author

prezha commented Oct 31, 2020

plz check lint

@medyagh, yes, the lint error here was expected - please see:

#9526 (comment) above:

this one should be ok to merge now (lint error hack/update/kubernetes_version/update_kubernetes_version.go:76:32: GHVersions not declared by package update is covered in pr #9529)

and

#9529 (comment):

@tstromberg similar as with pr #9532:

the conflicts were due to the pr #9464 (the original one - before i've split it into several prs) got merged first, and i had a couple of changes made in those individual prs; essentially my fault, i should have had closed the original pr right after the split (but left it just in case anyone wanted to compare)...

i've fixed this one now but had to also add here the base library from pr #9526 to resolve dependencies, should be ok to merge this as well as pr #9526 (i'll resolve the conflicts there if any)

we can:

or if you have a better proposal - please share your thoughts

@prezha prezha requested a review from medyagh October 31, 2020 01:29
@prezha prezha closed this Nov 11, 2020
@prezha prezha force-pushed the hack-update--base-libs branch from 296f7d4 to 6b287ef Compare November 11, 2020 00:44
@k8s-ci-robot k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 11, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: prezha
To complete the pull request process, please assign after the PR has been reviewed.
You can assign the PR to them by writing /assign 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

@prezha
Copy link
Contributor Author

prezha commented Nov 11, 2020

this one got automatically closed after merging #9529 and rebasing, as explained in #9529 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants