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

Knative Serving Deployment #454

Merged
merged 14 commits into from
Jun 5, 2020

Conversation

lholmquist
Copy link
Member

This PR starts to add the ability for nodeshift to deploy a knative serving service.

It is a work in progress, but the happy path works here.

using the nodejs-rest-http starter and code ready containers with the serverless operator installed and setup. i was able to do nodeshift --knative=true with out having to change the starter app.

For a deploy, the first part(build) is unchanged, the "apply-resource" part, which is the second phase of a nodeshift deploy, has been updated to only create a knative serving service that points to the image that was just built in the first step.

If an application has a .nodeshift directory with resource files that are not related to knative, they are ignored

@lholmquist lholmquist marked this pull request as ready for review June 3, 2020 15:23
@lholmquist
Copy link
Member Author

i know there is a lot to review here, but i'm adding this feature as experimental, so we can get it in and then iterate on it

Copy link
Member

@lance lance left a comment

Choose a reason for hiding this comment

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

LGTM with a handful of nits

README.md Outdated Show resolved Hide resolved
bin/nodeshift Outdated Show resolved Hide resolved
bin/nodeshift Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
lib/goals/resource.js Outdated Show resolved Hide resolved
lib/nodeshift-config.js Show resolved Hide resolved
lib/nodeshift-config.js Show resolved Hide resolved
@lholmquist
Copy link
Member Author

@lance i think I addressed the nits

@lholmquist lholmquist requested a review from lance June 5, 2020 15:06
@lholmquist
Copy link
Member Author

@lance looks like i got them all

Copy link
Member

@lance lance left a comment

Choose a reason for hiding this comment

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

LGTM

@lholmquist lholmquist merged commit 88eed5d into nodeshift:master Jun 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants