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.

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

Make deploy to print out install yaml files #282

Merged
merged 2 commits into from
Jun 4, 2019

Conversation

alenkacz
Copy link
Contributor

@alenkacz alenkacz commented May 31, 2019

What type of PR is this?
/kind enhancement
/kind infrastructure

What this PR does / why we need it:
Fixes #

Which issue(s) this PR fixes:

Fixes #222
It is just first step, but it should be enough to consider that issue fixed. The follow up will be #280 where we'll introduce helm integration as well as kudoctl command to print out the yaml

To prove that it works, this is from my machine

 kudo git:(master) ✗ make deploy | kubectl apply -f -
namespace/kudo-system created
customresourcedefinition.apiextensions.k8s.io/frameworks.kudo.k8s.io created
customresourcedefinition.apiextensions.k8s.io/frameworkversions.kudo.k8s.io created
customresourcedefinition.apiextensions.k8s.io/instances.kudo.k8s.io created
customresourcedefinition.apiextensions.k8s.io/planexecutions.kudo.k8s.io created
clusterrole.rbac.authorization.k8s.io/kudo-manager-role created
clusterrolebinding.rbac.authorization.k8s.io/kudo-manager-rolebinding created
secret/kudo-webhook-server-secret created
service/kudo-controller-manager-service created
statefulset.apps/kudo-controller-manager created

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

make install-crds was removed, make deploy now prints out the yaml files. New usage is `make deploy | kubectl apply -f -`

@alenkacz alenkacz requested a review from gerred as a code owner May 31, 2019 10:55
@gerred
Copy link
Member

gerred commented May 31, 2019

This is great. lgtm!

Copy link
Contributor

@joerg84 joerg84 left a comment

Choose a reason for hiding this comment

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

Do we consider the changes to the Makefile as external API changes?
I am just concerned that we might break some slides/demos which are not under our control.
As of right now, I don't believe it matters too much but we should fix that at some point.

@gerred
Copy link
Member

gerred commented Jun 3, 2019

I don't consider Makefile changes as external API changes. Maybe today, but only as "developer as the external user". This should be geared to development process.

Not that I'm hugely a fan of Makefiles, would prefer some other job running system but that's what we have with Go unless we were to use https://github.com/magefile/mage or something (which I am not in favor of at this time).

We've talked about kubectl kudo init as a later step 2 that WOULD be the external API that serves the same functionality. For v0.2.0, we're just going to go with "installation instructions" instead, and all of those manifests will be available in the release tarball.

@joerg84
Copy link
Contributor

joerg84 commented Jun 3, 2019

@gerred I agree that it shouldn't be, but looking at our slide decks and demos currently it is. That is the reason why I would just keep an eye out for that.
Furthermore, we should at one point document our external API and also our policy about changes.
Having said that, I already approved the PR so no objections here :-)

@gerred
Copy link
Member

gerred commented Jun 3, 2019

Good point. @mattj-io this affects you as well. We'll have this all documented in the v0.2.0 release notes...in fact, I'm going to make a label so that this isn't forgotten.

@gerred gerred added this to the v0.2.0 milestone Jun 3, 2019
@alenkacz alenkacz merged commit f0d3b3c into master Jun 4, 2019
@gkleiman gkleiman deleted the av/print-out-install-yaml branch June 4, 2019 16:03
@porridge porridge added release/highlight This PR is a highlight for the next release and removed release-highlight labels Mar 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release/highlight This PR is a highlight for the next release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Just one command to install kudo into cluster
4 participants