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.

Sign up for GitHub

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

Jump to bottom

*: make go/v3 the default plugin #4307

Merged
merged 1 commit into from
Dec 16, 2020
Merged

*: make go/v3 the default plugin #4307

merged 1 commit into from
Dec 16, 2020

Conversation

estroz
Copy link
Member

@estroz estroz commented Dec 12, 2020 •
edited
Loading

Description of the change:

  • hack/generate/samples: update webhook marker and a few other nits
  • internal/cmd/operator-sdk/cli: make go/v3 the default

Motivation for the change: now that go/v3 is stable, it can be the default plugin. This is not a breaking change because go/v2 is still supported.

Checklist

If the pull request includes user-facing changes, extra documentation is required:

@estroz
Copy link
Member Author

estroz commented Dec 12, 2020

Waiting on kubernetes-sigs/kubebuilder#1882

/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 12, 2020
@estroz estroz force-pushed the feature/go-v3-default branch from 29cbd64 to fa190d8 Compare December 14, 2020 22:02
@estroz
Copy link
Member Author

estroz commented Dec 14, 2020

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 14, 2020
@estroz estroz added this to the v1.3.0 milestone Dec 14, 2020
@estroz
Copy link
Member Author

estroz commented Dec 15, 2020

Blocked by #4315

@estroz estroz added area/testing Issue related to testing the operator-sdk and subcomponents blocked labels Dec 15, 2020
@camilamacedo86
Copy link
Contributor

camilamacedo86 commented Dec 15, 2020 •
edited
Loading

HHi @estroz,

It fails with the error:

warning: error when running kustomize build: kustomize build config/default failed with error: (exec: "kustomize": executable file not found in $PATH)

So, we might need to check if we have not an issue with the makefile target. The kb tests might install it globally and did not get it.

https://github.com/operator-framework/operator-sdk/pull/4307/checks?check_run_id=1553378087#step:5:427

camilamacedo86
camilamacedo86 reviewed Dec 15, 2020
View reviewed changes
changelog/fragments/go-v3-default.yaml
incompatible with projects created by go/v2. You are not required to upgrade
and your go/v2 project will continue to work with new operator-sdk versions.
If you wish to upgrade, check out the upstream
[migration guide](https://master.book.kubebuilder.io/migration/plugin/plugins.html).
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO we need to add here to update the layout from layout: go.kubebuilder.io/v2 to layout: go.kubebuilder.io/v3-alpha
Then, I think would be better the kb link at the end for users are able to look for further info.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd rather defer completely to upstream docs for this migration, since you may have to make some other changes before upgrading your project config file's layout key.

camilamacedo86
camilamacedo86 approved these changes Dec 15, 2020
View reviewed changes
Copy link
Contributor

@camilamacedo86 camilamacedo86 left a comment

Choose a reason for hiding this comment

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

Just add a few nits. We might want to wait for v3 stable in the kb as well before gets it merged.
However, /lgtm

@estroz estroz removed the area/testing Issue related to testing the operator-sdk and subcomponents label Dec 15, 2020
@estroz estroz force-pushed the feature/go-v3-default branch from fa190d8 to 0490ff0 Compare December 15, 2020 09:04
@estroz
Copy link
Member Author

estroz commented Dec 15, 2020

@camilamacedo86 the problem is TestContext.CleanupManifests() uses the global kustomize binary while go/v3 installs kustomize to ./bin/kustomize. That function needs to be updated to consider a local kustomize path. In the meantime I've updated the cleanup step to use make undeploy, and added a comment about future refactoring.

@estroz estroz removed the blocked label Dec 15, 2020
@estroz estroz force-pushed the feature/go-v3-default branch from 0490ff0 to 59bba07 Compare December 15, 2020 19:42
@estroz estroz merged commit f89a2c3 into operator-framework:master Dec 16, 2020
@estroz estroz deleted the feature/go-v3-default branch December 16, 2020 21:30
reinvantveer pushed a commit to reinvantveer/operator-sdk that referenced this pull request Feb 5, 2021
*: make go/v3 the default plugin (operator-framework#4307)
8a16d2f 
Signed-off-by: reinvantveer <rein.van.t.veer@geodan.nl>
rearl-scwx pushed a commit to rearl-scwx/operator-sdk that referenced this pull request Feb 5, 2021
@rearl-scwx
*: make go/v3 the default plugin (operator-framework#4307)
1c45207 
Signed-off-by: rearl <rearl@secureworks.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@camilamacedo86 camilamacedo86 camilamacedo86 approved these changes

@fabianvf fabianvf Awaiting requested review from fabianvf

@theishshah theishshah Awaiting requested review from theishshah

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v1.3.0
Development

Successfully merging this pull request may close these issues.
3 participants
@estroz @camilamacedo86 @openshift-ci-robot