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

Fix conversion to OpenShift (invalid DeploymentConfig) #126

Merged
merged 2 commits into from
Sep 5, 2016

Conversation

kadel
Copy link
Member

@kadel kadel commented Aug 29, 2016

fixes #124 by add missing conversion to versioned object when saving to files

I've also added commit that fixes #130

@janetkuo
Copy link
Member

Mind adding a test to prevent regression?

@kadel
Copy link
Member Author

kadel commented Aug 30, 2016

there are already tests that should catch this, but they are not working properly :-(
see #125

edit:
no this test couldn't catch that even if it would work, because it uses --stdout and that bug was only when converting to files.

I've created issue to fix the tests: #129

@surajssd
Copy link
Member

@kadel to address this issue in #73 I added a function called sortServices at that time somehow it seems to be out of code :(

This function was created because @ngtuna asked for it #94 (comment)

Link to function in diff
04d1d65#diff-41d801ef80f1858d5e8e9695667e4dafR1302

}
//make sure that Serices are first in the List
switch object.(type) {
case *api.Service:
Copy link
Member

Choose a reason for hiding this comment

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

To simplify this switch you can also use object.GetObjectKind().GroupVersionKind().Kind == "Service" to do the checks.

@kadel
Copy link
Member Author

kadel commented Aug 30, 2016

@kadel to address this issue in #73 I added a function called sortServices at that time somehow it seems to be out of code :(

This function was created because @ngtuna asked for it #94 (comment)

Link to function in diff
04d1d65#diff-41d801ef80f1858d5e8e9695667e4dafR1302

ah, I missed that, I'll move that function to Kubernetes Transformer and use that

@kadel kadel force-pushed the fix-openshift-output branch 2 times, most recently from ec58677 to 1cdfdaf Compare August 30, 2016 13:28
add missing conversion to versioned object when saving to files
Sorts objects in Transform function, this should
make sure that Services are first everywhere as
long as we keep order in the slice that Transform returns.

fixes kubernetes#130
@kadel kadel force-pushed the fix-openshift-output branch from 1cdfdaf to e2da0f0 Compare September 2, 2016 08:09
@kadel
Copy link
Member Author

kadel commented Sep 2, 2016

Heyo, can please someone review this?
@surajssd @ngtuna @janetkuo ?

@sebgoa
Copy link
Contributor

sebgoa commented Sep 2, 2016

+1 looks ok, but did not test

@kadel kadel merged commit 7e91770 into kubernetes:master Sep 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

services should be first in List OpenShift conversoin - invalid DeploymentConfig
4 participants