🌱 [WIP] Upgrade the golangci-lint

[Darshnadas]

What this PR does / why we need it:

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 #
Please have a look into this PR, as there are many things showing as unidentified. In accordance with #6163

[k8s-ci-robot]

Welcome @Darshnadas!

It looks like this is your first PR to kubernetes-sigs/cluster-api 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/cluster-api has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[k8s-ci-robot]

Hi @Darshnadas. Thanks for your PR.

I'm waiting for a kubernetes-sigs 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]

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

[fabriziopandini]

/ok-to-test

[killianmuldoon]

/retitle 🌱 [WIP] Upgrade the golangci-lint

[killianmuldoon]

Thanks for this @Darshnadas - it looks like there's a couple of small errors to fix - removing new lines from util/container/image.go Could you make those changes?

[Darshnadas]

Sure @killianmuldoon what changes do I need to do?

[killianmuldoon]

Are you able to run make lint locally? It should point out the errors on your machine currently. After that you should be able to make changes - I think it's just removing lines in util/container/image.go - and update this PR.

If you don't know what changes to make please run make lint locally and post the results here. You can also reach out to me on the Kubernetes slack if that's better 😄

[Darshnadas]

I'll run make lint and reach out to you on slack @killianmuldoon :) Thank you for helping

[Darshnadas]

Hi @killianmuldoon I was not able to send you a request in slack, how can I connect to you?

[killianmuldoon]

@Darshnadas let's keep it here for now. Did you run make lint and fix the problems it found?

[Darshnadas]

yes I ran make lint I am pasting it in a Pastebin and giving it here https://pastebin.com/kf9ZhuHG, please review it. I am not able to understand as it says "missing type.."

[killianmuldoon]

I'm not sure what the issue is here - but the linter is showing a lot of errors that aren't on my system. Can you show the output of go version and make generate-modules

[Darshnadas]

Shall I run go version and make generate-modules?

[killianmuldoon]

Yes, please run them and let me know the output and if there are any errors.

[Darshnadas]

Yes, please run them and let me know the output and if there are any errors.

I ran the given two commands, pasting the output here:-

➜  cluster-api git:(lint) go version          
go version go1.16.4 linux/amd64
➜  cluster-api git:(lint) make generate-modules
make: *** No rule to make target 'generate-modules'.  Stop.

[killianmuldoon]

@Darshnadas Can you update your repo and try to run all of the commands - make generate-modules and make lint again? Looking at your fork your repo is really out of date. You can try to do git rebase main. If that gives you some conflicts you can try to do git checkout main and git pull origin/main (assuming that kubernetes-sigs/cluster-api) is origin for your local git repo.

[Darshnadas]

@killianmuldoon oh! okay

[Darshnadas]

@killianmuldoon I tried doing it but I got some errors, I did some googling and tried the above command, I am a beginner in git hence, I am not sure how git works.

➜  cluster-api git:(lint) git checkout main                        
Switched to branch 'main'
Your branch is up to date with 'origin/main'.
➜  cluster-api git:(main) git pull origin/main  
fatal: 'origin/main' does not appear to be a git repository
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.
➜  cluster-api git:(main) git remote -v
origin	[email protected]:Darshnadas/cluster-api.git (fetch)
origin	[email protected]:Darshnadas/cluster-api.git (push)
➜  cluster-api git:(main) git push origin main      
Everything up-to-date

I"ll try running the make lint command again

[killianmuldoon]

@Darshnadas There may be a "Fetch Upstream" button on your git repo that will automatically update your fork with the cluster api repo. It looks like this on my repo.

If you press this and then run git pull again you might be able to update. Otherwise I think you'll have to spend some time getting to know your way around git.
This is a really great tutorial - https://learngitbranching.js.org/
There's good documentation from Github here: https://docs.github.com/en

[Darshnadas]

@killianmuldoon Understood, I did the fetch upstream and tried doing the git pull command again but it gives the same error. I will read the given links by you get back to you, also if possible can you say by what name shall I search you in slack?

[killianmuldoon]

@Darshnadas I think it might be better to keep most issues on github - but if you have questions you should ask generally in the cluster-api channel on Kubernetes slack.

What do you think about closing this PR until you resolve some of the git issues?

[Darshnadas]

@killianmuldoon I got your point, since I am a beginner at cluster-api, I'll try asking questions in the channel itself, and for this PR I have changed the version number just, I might be able to update the repo and try running the make lint command, or I can unassign and let some other folk start working, I need few days to get familiar with git. Is it okay if I take a few days time?

[killianmuldoon]

@Darshnadas Sure thing - there's no major rush, I just don't want you to feel under pressure to get this done when you're starting out and learning 🙂 . I'm looking forward to seeing you in the channel!

[Darshnadas]

@Darshnadas Sure thing - there's no major rush, I just don't want you to feel under pressure to get this done when you're starting out and learning slightly_smiling_face . I'm looking forward to seeing you in the channel!

Thank you, will get back to you / in the channel, also if I did fetch upstream then why is it still showing this same error? any idea?

[killianmuldoon]

I think the issue might be what git branch you're on when you're running make lint. With git it's always a good idea to check what context you're in to make sure you're running commands in the right branch.

If you run git status it will show you what branch you're on. My guess is that you ran github fetch upstream on your main branch on github but you're running make lint on the lint branch you created for this PR.

[Darshnadas]

I think the issue might be what git branch you're on when you're running make lint. With git it's always a good idea to check what context you're in to make sure you're running commands in the right branch.

If you run git status it will show you what branch you're on. My guess is that you ran github fetch upstream on your main branch on github but you're running make lint on the lint branch you created for this PR.

yes, this is what I did, so I need to run fetch upstream in my lint branch am I right?

[killianmuldoon]

/ok-to-test

[killianmuldoon]

When you run make lint locally do you get the same errors as before?

[Darshnadas]

yes, has missing type errors also another error

util/container/image.go:21:1: Expected '\t', Found '\n' at util/container/image.go[line 21,col 1] (gci)

I think I might have to change in image.go file something, shall I change as the error is pointing?

[Darshnadas]

There are many undefined errors coming from different files

[killianmuldoon]

There are many undefined errors coming from different files

Now that you're on main can you try to run make generate-modules? This might be able to fix your missing types. If there's some deeper issue the output of that command might point to it so you can get it fixed.

util/container/image.go:21:1: Expected '\t', Found '\n' at util/container/image.go[line 21,col 1] (gci)

The above error is the only one that I'm having. I think you can safely remove the new line - \n at that line number in that file. I think there should be a second one which will be reported once you've fixed the first.

[Darshnadas]

Okay @killianmuldoon understood

[Darshnadas]

@killianmuldoon I'll be out of town for the next 2 days, I will get back here once I run the make generate-modules

[Darshnadas]

hi @killianmuldoon, I ran make generate-modules

cluster-api git:(main) make generate-modules
make: *** No rule to make target 'generate-modules'.  Stop.

nd this came the output

[Darshnadas]

Also I ran make lint and there was no util/container/image.go:21:1: Expected '\t', Found '\n' at util/container/image.go[line 21,col 1] (gci) error but still different missing types from different files pointed

[killianmuldoon]

Can you try running the following commands and pasting the commands and their output in a pastebin?

Run all commands from the cluster api directory:

git checkout main
git pull
make generate-modules
make lint-fix

[Darshnadas]

sure @killianmuldoon

[Darshnadas]

@killianmuldoon here is the Pastebin https://pastebin.com/XcuDeyGq, can you please review it?

[killianmuldoon]

There seems to be a number of unrelated build errors going on here which I'm not sure how to fix. Can you try the following to be sure we've got a full clean build?
Start from whichever directory you keep your go projects.

git clone https://github.com/kubernetes-sigs/cluster-api

cd cluster-api

make lint

make test

If the above passes without error try to update the linter version again and re-run make-lint to see if it produces the same errors. If the above clean version of the code show the same issues we'll have to tackle that problem before looking to the linter.

[Darshnadas]

okay @killianmuldoon I'll get back here once I do the above steps

[killianmuldoon]

Thanks @Darshnadas! I think we should close this PR at this stage - the golangci-lint has had a couple of version bumps since this was opened and there's a new pr at #6335.

Please reach out on the slack cluster-api channel so we can try to resolve your build issues though so it will be easier for you to contribute in future.

[killianmuldoon]

/close

[k8s-ci-robot]

@killianmuldoon: Closed this PR.

In response to this:

/close

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.

[Darshnadas]

@killianmuldoon sure, thank you