-
Notifications
You must be signed in to change notification settings - Fork 243
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
Deploy Command for OCP #3478
Deploy Command for OCP #3478
Conversation
…o parse_dockerfile
…he projects source
Signed-off-by: Steven Groeger <groeges@uk.ibm.com>
Signed-off-by: Steven Groeger <groeges@uk.ibm.com>
Signed-off-by: Steven Groeger <groeges@uk.ibm.com>
- Downloads Dockerfile into memory rather than onto the hosts machine - Validates if a Dockerfile is present in the project source, if one isn't specified by the devfile - Validates the contents of the Dockerfile (that there is a FROM on the first non-comment/non-whitespace line) - Validates the tag passed as an arg conforms to the correct character-set - Copies all project source files into the container rather than new/updated - Additional Testing - More appropriate error/warning messages
Co-authored-by: Cameron McWilliam <cam.mcwilliam@me.com>
* Add test List URL function Signed-off-by: Steven Groeger <groeges@uk.ibm.com> * Update logging within maifest deploy Signed-off-by: Steven Groeger <groeges@uk.ibm.com> * Add preliminary support for multiple manifest in single yaml Signed-off-by: Steven Groeger <groeges@uk.ibm.com> * Update service manifest with ClusterIP value before update. Signed-off-by: Steven Groeger <groeges@uk.ibm.com> * Merge deployDelete code and update to support multiple doc in yaml Signed-off-by: Steven Groeger <groeges@uk.ibm.com> * Fix source formatting Signed-off-by: Steven Groeger <groeges@uk.ibm.com> * Add some extra checks on waitManifestDeployComplete function Signed-off-by: Steven Groeger <groeges@uk.ibm.com>
alpha.deployment-manifest Signed-off-by: Steven Groeger <groeges@uk.ibm.com>
Signed-off-by: Steven Groeger <groeges@uk.ibm.com>
Signed-off-by: Steven Groeger <groeges@uk.ibm.com>
* Remove CommonPushOptions and do some clean up * Move validation, add better user experience, remove devfile.yaml flag
Signed-off-by: Steven Groeger <groeges@uk.ibm.com>
quick question how is this PR different from #3751? |
@girishramnani this PR came first and it introduces all the code for the #3751 is built on top of this PR and just extends the odo deploy function to also perform unprivileged builds on kubernetes clusters (using kaniko) |
Will try, thanks! |
/retest |
/retest |
@EnriqueL8: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
@EnriqueL8, I built your branch from 51a5d36 on Win10. One thing that surprised me is that the keys change after doing the Odo changed my devfile keys from:
Is this expected? I noticed too that when I did |
This shouldn't be the case - will investigate further |
Opened #3968 to address the loss of the unrecognized properties on rewrite.. and maybe this PR was planning on dealing with this sepearately, and making them "recognized" within the parser's odo-internal representation of the schema. |
Just to clarify, this doesn't attempt to implement the build args described here: devfile/api#51 (comment) correct?
|
// if it does, then check we have an port setup in env.yaml | ||
do.DeploymentPort = 0 | ||
if bytes.Contains(manifestBytes, []byte("{{.PORT}}")) { | ||
deploymentPort, err := do.EnvSpecificInfo.GetPortByURLKind(envinfo.ROUTE) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this maybe should be revisited. We'd like to enable the use case where, on OCP, you can push without doing odo url create
, which means there will only be a devfile entry, but no env.yaml entry for the Url.
Another thought.. (and maybe some of this should be moved to follow-up issue(s)).. it seems by default the tag is based on the current project / NS, not the one embedded in the project env.yaml (whereas with a push the pod is created in the env.yaml NS). |
@@ -142,6 +142,19 @@ jobs: | |||
# - travis_wait make test-cmd-docker-devfile-url-pushtarget | |||
# These tests need cluster login as they will be interacting with a Kube environment | |||
- odo login -u developer | |||
- travis_wait make test-cmd-devfile-catalog |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAIK we are already running the whole devfile target under https://github.com/openshift/odo/blob/master/Makefile#L374-L377, its just duplicating the jobs.
@EnriqueL8: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
@EnriqueL8: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
thank you for this PR but it seems to have diverged, we can come back to this feature once we are ready, hence closing |
What type of PR is this?
/kind feature
What does does this PR do / why we need it:
This PR implements
odo deploy
andodo deploy delete
see proposal #3368.Which issue(s) this PR fixes:
Implements #3300
How to test changes / Special notes to the reviewer:
odo create
- I would recommend choosing a stack that exposes a server associated with it or create a sample nodejs express server in your project. So that you container doesn't crash withCrashLoopBackOff
and you can access a running app with the url.alpha.build-dockerfile
andalpha.deployment-manifest
(See proposal). Use this as manifest.odo url create
odo deploy --tag image-registry.openshift-image-registry.svc:5000/<namespace>/<image-name>:<tag>