openshift / assisted-installer

Apache License 2.0
69 stars 98 forks source link

NO-ISSUE: Update Dockerfile to use `go.uber.org/mock` #911

Closed jhernand closed 1 week ago

jhernand commented 1 month ago

The change to use go.uber.org/mock instead of the deprecated github.com/golang/mock was was mostly done in the past. But the Dockerfile wasn't updated to use the new mockgen command. As a result if one installs the mockgen with the command from the dockerfile and then runs make generate the resulting code doesn't build:

$ grep mockgen Dockerfile.assisted-installer-build
  go install github.com/golang/mock/mockgen@v1.6.0 && \

$ go install github.com/golang/mock/mockgen@v1.6.0

$ make generate
go generate ..
make format
make[1]: Entering directory '/files/projects/assisted-installer/repository'
make[1]: Leaving directory '/files/projects/assisted-installer/repository'

$ make build
CGO_ENABLED=0  go build -o build/installer src/main/main.go
src/main/drymock/dry_mode_k8s_mock.go:308:36: cannot use mockController (variable of type *"go.uber.org/mock/gomock".Controller) as *"github.com/golang/mock/gomock".Controller value in argument to k8s_client.NewMockK8SClient
make: *** [Makefile:68: installer] Error 1

To avoid this issue this patch updates the Dockerfile to use the go.uber.org/mock package.

There are also some minor white space changes introduced by running go generate.

openshift-ci-robot commented 1 month ago

@jhernand: This pull request explicitly references no jira issue.

In response to [this](https://github.com/openshift/assisted-installer/pull/911): >The change to use `go.uber.org/mock` instead of the deprecated `github.com/golang/mock` was was mostly done in the past. But the _Dockerfile_ wasn't updated to use the new `mockgen` command. As a result if one installs the `mockgen` with the command from the dockerfile and then runs `make generate` the resulting code doesn't build: > >``` >$ grep mockgen Dockerfile.assisted-installer-build > go install github.com/golang/mock/mockgen@v1.6.0 && \ > >$ go install github.com/golang/mock/mockgen@v1.6.0 > >$ make generate >go generate .. >make format >make[1]: Entering directory '/files/projects/assisted-installer/repository' >make[1]: Leaving directory '/files/projects/assisted-installer/repository' > >$ make build >CGO_ENABLED=0 go build -o build/installer src/main/main.go >src/main/drymock/dry_mode_k8s_mock.go:308:36: cannot use mockController (variable of type *"go.uber.org/mock/gomock".Controller) as *"github.com/golang/mock/gomock".Controller value in argument to k8s_client.NewMockK8SClient >make: *** [Makefile:68: installer] Error 1 >``` > >To avoid this issue this patch updates the _Dockerfile_ to use the `go.uber.org/mock` package. > >There are also some minor white space changes introduced by running `go generate`. Instructions for interacting with me using PR comments are available [here](https://prow.ci.openshift.org/command-help?repo=openshift%2Fassisted-installer). If you have questions or suggestions related to my behavior, please file an issue against the [openshift-eng/jira-lifecycle-plugin](https://github.com/openshift-eng/jira-lifecycle-plugin/issues/new) repository.
openshift-ci[bot] commented 1 month ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jhernand

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: - ~~[OWNERS](https://github.com/openshift/assisted-installer/blob/master/OWNERS)~~ [jhernand] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 57.85%. Comparing base (2bb58e1) to head (bfb7176). Report is 8 commits behind head on master.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/openshift/assisted-installer/pull/911/graphs/tree.svg?width=650&height=150&src=pr&token=X6PHZZFE9E&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openshift)](https://app.codecov.io/gh/openshift/assisted-installer/pull/911?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openshift) ```diff @@ Coverage Diff @@ ## master #911 +/- ## ========================================== + Coverage 55.70% 57.85% +2.14% ========================================== Files 15 15 Lines 3208 3566 +358 ========================================== + Hits 1787 2063 +276 - Misses 1249 1322 +73 - Partials 172 181 +9 ``` | [Flag](https://app.codecov.io/gh/openshift/assisted-installer/pull/911/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openshift) | Coverage Δ | | |---|---|---| | [](https://app.codecov.io/gh/openshift/assisted-installer/pull/911/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openshift) | `55.70% <ø> (?)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openshift#carryforward-flags-in-the-pull-request-comment) to find out more. [see 3 files with indirect coverage changes](https://app.codecov.io/gh/openshift/assisted-installer/pull/911/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openshift)
eranco74 commented 1 month ago

/lgtm

jhernand commented 1 week ago

/remove-label downstream-change-needed

openshift-ci[bot] commented 1 week ago

@jhernand: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
openshift-bot commented 1 week ago

[ART PR BUILD NOTIFIER]

Distgit: ose-agent-installer-orchestrator This PR has been included in build ose-agent-installer-orchestrator-container-v4.18.0-202410251810.p0.g628fadb.assembly.stream.el9. All builds following this will include this PR.

openshift-bot commented 1 week ago

[ART PR BUILD NOTIFIER]

Distgit: ose-agent-installer-csr-approver This PR has been included in build ose-agent-installer-csr-approver-container-v4.18.0-202410251810.p0.g628fadb.assembly.stream.el9. All builds following this will include this PR.