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

Include the operator-sdk bundle as repo payload #175

Merged
merged 1 commit into from
Jan 27, 2023
Merged

Include the operator-sdk bundle as repo payload #175

merged 1 commit into from
Jan 27, 2023

Conversation

ArangoGutierrez
Copy link
Contributor

It has become common practice to include the bundle folder in the repo this patch is generated by running "make bundle", moving forward updating the bundle will be part of the release process.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jan 17, 2023
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ArangoGutierrez

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:

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

@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jan 17, 2023
@ArangoGutierrez ArangoGutierrez added this to the v0.6.0 milestone Jan 17, 2023
@ArangoGutierrez
Copy link
Contributor Author

Part of #174

@ArangoGutierrez
Copy link
Contributor Author

/assign @marquiz

marquiz
marquiz reviewed Jan 19, 2023
View reviewed changes
Copy link
Contributor

@marquiz marquiz left a comment

Choose a reason for hiding this comment

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

Thanks @ArangoGutierrez.

Could you also include the scripts, makefile rule(s) or whatever is needed to update the bundle? And update the release process in ´.github/ISSUE_TEMPLATE/release.md` to include this (if it's needed)?

Also, you could split this PR into two commits: the first one introducing the tooling to update the bundle (Dockerfile et al) and the second commit being the "auto-generated" content, i.e. new content added by running that tooling

@ArangoGutierrez
Copy link
Contributor Author

Hey Markus thanks for the review, sure to the first part, working on that now, but the scond part is already there, this PR is basically generated by running make bundle VERSION=x.y.z so the tooling is there already.

@marquiz
Copy link
Contributor

marquiz commented Jan 24, 2023

Are you planning to address the documentation in this PR?

BTW, just out of curiosity, I tried running make bundle locally. Of course, it didn't work and errored out because of incompatible version of operator-sdk. Any thoughts how to address these annoying skew problems? Doing containerized build for make bundle? At least we need to document what versions of tools are needed

@ArangoGutierrez
Copy link
Contributor Author

Are you planning to address the documentation in this PR?

BTW, just out of curiosity, I tried running make bundle locally. Of course, it didn't work and errored out because of incompatible version of operator-sdk. Any thoughts how to address these annoying skew problems? Doing containerized build for make bundle? At least we need to document what versions of tools are needed

Are you planning to address the documentation in this PR?

BTW, just out of curiosity, I tried running make bundle locally. Of course, it didn't work and errored out because of incompatible version of operator-sdk. Any thoughts how to address these annoying skew problems? Doing containerized build for make bundle? At least we need to document what versions of tools are needed

Are you planning to address the documentation in this PR?

BTW, just out of curiosity, I tried running make bundle locally. Of course, it didn't work and errored out because of incompatible version of operator-sdk. Any thoughts how to address these annoying skew problems? Doing containerized build for make bundle? At least we need to document what versions of tools are needed

make bundle should work for you now

I will document in a following PR, I want to work towards adding HELM, then some docs, then cut release, that's the plan before Feb 🤞🏽

@marquiz
Copy link
Contributor

marquiz commented Jan 26, 2023

make bundle should work for you now

Hmm 🤨

$ make bundle
/home/marquiz/go/git/node-feature-discovery-operator/review/bin/controller-gen "crd" rbac:roleName=manager-role webhook paths="./..." output:crd:artifacts:config=config/crd/bases
operator-sdk generate kustomize manifests -q
Error: unknown shorthand flag: 'q' in -q
Usage:
  operator-sdk generate [command]
...

@ArangoGutierrez
Copy link
Contributor Author

make bundle should work for you now

Hmm raised_eyebrow

$ make bundle
/home/marquiz/go/git/node-feature-discovery-operator/review/bin/controller-gen "crd" rbac:roleName=manager-role webhook paths="./..." output:crd:artifacts:config=config/crd/bases
operator-sdk generate kustomize manifests -q
Error: unknown shorthand flag: 'q' in -q
Usage:
  operator-sdk generate [command]
...

[eduardo@fedora rhcos]$ operator-sdk version
operator-sdk version: "v1.26.0-7-ga5d933b5", commit: "a5d933b5e8d729c9c3f9f1790f61053cf37dfafd", kubernetes version: "v1.25.0", go version: "go1.19.5", GOOS: "linux", GOARCH: "amd64"

...?

@marquiz
Copy link
Contributor

marquiz commented Jan 26, 2023

That was my point. I've have my share of wrong tool/library versions. So could you document e.g. in the commit message what version of operator-sdk was used

@ArangoGutierrez
Include the operator-sdk bundle as repo payload
72e2477 
It has become common practice to include the bundle folder in the repo
this patch is generated by running "make bundle", moving forward
updating the bundle will be part of the release process.

Generated with:
- operator-sdk version: "v1.26.0"
- kubernetes version: "v1.25.0"
- go version: "go1.19.5"
- GOOS: "linux"
- GOARCH: "amd64"
@ArangoGutierrez
Copy link
Contributor Author

@marquiz PTAL, commit now includes the version of dependencies to generate the bundle as is in this patch.

marquiz
marquiz reviewed Jan 27, 2023
View reviewed changes
Copy link
Contributor

@marquiz marquiz left a comment

Choose a reason for hiding this comment

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

Thanks @ArangoGutierrez for the update
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 27, 2023
@k8s-ci-robot k8s-ci-robot merged commit 180644f into kubernetes-sigs:master Jan 27, 2023
@ArangoGutierrez ArangoGutierrez mentioned this pull request Jan 30, 2023
Closed
5 tasks
@ArangoGutierrez ArangoGutierrez deleted the helm_init branch March 13, 2023 10:33
@ArangoGutierrez ArangoGutierrez mentioned this pull request Mar 14, 2023
Closed
16 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marquiz marquiz marquiz left review comments

@zvonkok zvonkok Awaiting requested review from zvonkok

Assignees

@marquiz marquiz

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
v0.6.0
Development

Successfully merging this pull request may close these issues.
3 participants
@ArangoGutierrez @k8s-ci-robot @marquiz