Adding --controller flag in up & down
#868
Conversation
k8s-ci-robot
added
cncf-cla: yes
surajnarwade
changed the title
--controller flag in up & down--controller flag in up & down
k8s-ci-robot
added
the
do-not-merge/work-in-progress
|
@surajnarwade Need tests 👍 |
| @@ -797,6 +797,21 @@ func (k *Kubernetes) Deploy(komposeObject kobject.KomposeObject, opt kobject.Con | |||
| return err | |||
| } | |||
| log.Infof("Successfully created Deployment: %s", t.Name) | |||
|
|
|||
| case *extensions.DaemonSet: | |||
There was a problem hiding this comment.
Not too sure why this code is being added? Can you describe this at least within the commit description?
There was a problem hiding this comment.
Sure I will mention
There was a problem hiding this comment.
@cdrage , updated commit message
There was a problem hiding this comment.
please update GitHub description too...
surajnarwade
force-pushed
the
equal_up_convert
branch
from
c061f05
to
9308b06
Compare
To make `kompose up` & `kompose convert` equal in feature, This PR will add `--controller` flag for `kompose up` as well as `kompose down` so that user experience will be the same for `up` & `convert` Resolves kubernetes#798 since we are adding `--controller` to `up` and `down`, So respective code to deploy and undeploy also being added for `daemonset` and `replicationcontroller` Added tests for `--controller`
surajnarwade
force-pushed
the
equal_up_convert
branch
from
9308b06
to
126d982
Compare
|
@cdrage , review needed :) |
surajnarwade
changed the title
--controller flag in up & down--controller flag in up & down
k8s-ci-robot
removed
the
do-not-merge/work-in-progress
| @@ -103,6 +103,19 @@ test_k8s() { | |||
| sleep 2 # Sleep for k8s to catch up to deployment | |||
| echo -e "\n${RED}kompose down -f $f ${NC}\n" | |||
| ./kompose down -f $f | |||
| echo -e "\nTesting controller=daemonset key\n" | |||
There was a problem hiding this comment.
I think we should find an alternative way to write these integration tests (perhaps in a separate function? and only running the httpd.yaml test?) or even a separate file...
It's a bit messy at the moment and we really need to refactor everything (using e2e_test.go similar to our other project we're working on.
Do you mind opening an issue explaining that we should refactor our integration tests to use a Go file / make this better? As well as add a comment (and spacing above this line) with a # TODO: need to reorganize / refactor
Otherwise, this LGTM (for now).
There was a problem hiding this comment.
Thanks for adding issue as well.
Ready to merge ?
|
LGTM! Let's merge! |
To make
kompose up&kompose convertequal in feature, This PR willadd
--controllerflag forkompose upas well askompose downso that user experience will be the same for
up&convertResolves #798