Skip to content

Modify Makefile to install operator-sdk v0.12.0 if not installed#349

Closed
jan-est wants to merge 1 commit into
metal3-io:masterfrom
Nordix:hack-install-operator-sdk
Closed

Modify Makefile to install operator-sdk v0.12.0 if not installed#349
jan-est wants to merge 1 commit into
metal3-io:masterfrom
Nordix:hack-install-operator-sdk

Conversation

@jan-est
Copy link
Copy Markdown
Contributor

@jan-est jan-est commented Nov 27, 2019

Adds install.sh script for operator-sdk installation to hack folder

@metal3-io-bot
Copy link
Copy Markdown
Contributor

metal3-io-bot commented Nov 27, 2019

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: jan-est
To complete the pull request process, please assign hardys
You can assign the PR to them by writing /assign @hardys in a comment when ready.

The full list of commands accepted by this bot can be found here.

Details 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

@metal3-io-bot metal3-io-bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Nov 27, 2019
Jaakko-Os
Jaakko-Os reviewed Nov 27, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@Jaakko-Os Jaakko-Os left a comment •
edited
Loading

Choose a reason for hiding this comment

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

looks good!

maelk
maelk reviewed Nov 27, 2019
View reviewed changes
Comment thread Makefile Outdated
Comment thread Makefile Outdated
Comment thread hack/install.sh
Comment thread hack/install.sh Outdated
@jan-est jan-est force-pushed the hack-install-operator-sdk branch from 2d488af to a726e8c Compare November 28, 2019 08:41
@metal3-io-bot metal3-io-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Nov 28, 2019
maelk
maelk reviewed Nov 29, 2019
View reviewed changes
Comment thread Makefile Outdated
Comment thread hack/install.sh
Comment thread hack/install.sh Outdated
@jan-est jan-est force-pushed the hack-install-operator-sdk branch 3 times, most recently from 8ae61ff to a4c46e8 Compare December 2, 2019 07:44
zaneb
zaneb reviewed Dec 4, 2019
View reviewed changes
Comment thread hack/install.sh Outdated
curl -LO https://github.com/operator-framework/operator-sdk/releases/download/${REQUIRED_OPERATOR_SDK_VERSION}/operator-sdk-${REQUIRED_OPERATOR_SDK_VERSION}-x86_64-linux-gnu
chmod +x operator-sdk-${REQUIRED_OPERATOR_SDK_VERSION}-x86_64-linux-gnu
sudo mkdir -p /usr/local/bin/
sudo cp operator-sdk-${REQUIRED_OPERATOR_SDK_VERSION}-x86_64-linux-gnu /usr/local/bin/operator-sdk
Copy link
Copy Markdown
Member

@zaneb zaneb Dec 4, 2019

Choose a reason for hiding this comment

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

Is it necessary to use sudo here? You could just install in ${HOME}/bin/. Personally I installed the sdk using go get so it's in ~/go/bin/; that would be another way to do it (and is not specific to a particular operating system and CPU arch like downloading the tarball is, so it would work for e.g. Mac users as well).

Copy link
Copy Markdown
Member

@dhellmann dhellmann Dec 4, 2019

Choose a reason for hiding this comment

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

Yes, please use "go get" like we do for kustomize over in the cluster-api-provider repo https://github.com/metal3-io/cluster-api-provider-baremetal/blob/master/Makefile#L19

Copy link
Copy Markdown
Contributor Author

@jan-est jan-est Dec 12, 2019

Choose a reason for hiding this comment

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

I modified install.sh to install operator-sdk to ./bin folder

@jan-est jan-est force-pushed the hack-install-operator-sdk branch from a4c46e8 to 86de7a4 Compare December 12, 2019 11:34
@jan-est
Copy link
Copy Markdown
Contributor Author

jan-est commented Dec 12, 2019

@zaneb and @dhellmann sorry about my slow response with PR. I think this patch and right operator-sdk version should fix "make generate" problems issue mentioned in #360

@maelk maelk mentioned this pull request Dec 12, 2019
Closed
@longkb longkb mentioned this pull request Dec 13, 2019
Closed
longkb
longkb reviewed Dec 18, 2019
View reviewed changes
Comment thread hack/install.sh
# If local operator-sdk version is not required version
# or operator-sdk is not installed, install operator-sdk v0.12.0
if [ "${LOCAL_OPERATOR_SDK_VERSION}" != "${REQUIRED_OPERATOR_SDK_VERSION}" ] || [ -z "${LOCAL_OPERATOR_SDK_VERSION}" ]; then
mkdir -p ./bin
Copy link
Copy Markdown
Contributor

@longkb longkb Dec 18, 2019

Choose a reason for hiding this comment

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

Should we create .bin folder here? With this command, the new folder will be added into git status command after that.
How about $GOPATH/bin folder?

@zaneb
Copy link
Copy Markdown
Member

zaneb commented Dec 19, 2019

Is it necessary to require 0.12? This requires golang 1.13, which was only released 3 months ago and isn't in plenty of relatively recent distros (e.g. Fedora 30, Ubuntu 19.10).
(It's also extremely non-obvious that this would be the case, which may or may not be related to the problems people are having with it.)

@jan-est
Copy link
Copy Markdown
Contributor Author

jan-est commented Jan 10, 2020

@zaneb you might be right that operator-sdk version might not be the problem. It is better not to require golang 1.13 at this point. If anyone is not facing any problems with current operator-sdk we can drop this PR.

@zaneb
Copy link
Copy Markdown
Member

zaneb commented Jan 13, 2020

I mean, I'm definitely having problems (I used go get to install 0.8 or something and I can't get it to update to 0.11 due to dependency hell).
I just wonder if the real solution is to vendor the operator-sdk code like we do with other dependencies. I'd rather run code that I compiled myself, and I'm sure folks who are developing on non-Linux systems would appreciate that too.
Maybe switching to modules will make all of this easier... that reminds me to go review #367

@jan-est jan-est closed this Jan 15, 2020
@kashifest kashifest deleted the hack-install-operator-sdk branch October 5, 2023 06:39
iurygregory pushed a commit to iurygregory/baremetal-operator that referenced this pull request May 8, 2024
@openshift-merge-bot
Merge pull request metal3-io#349 from jgato/PreprovisioningImage-powe…
33cfa9f 
…roff-fix

OCPBUGS-33048: PreprovisioningImage should not be created on poweroff
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dhellmann dhellmann dhellmann left review comments

@zaneb zaneb zaneb left review comments

+3 more reviewers

@maelk maelk maelk left review comments

@longkb longkb longkb left review comments

@Jaakko-Os Jaakko-Os Jaakko-Os left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@jan-est @metal3-io-bot @zaneb @dhellmann @maelk @longkb @Jaakko-Os