Skip to content

Prefer legacy kinds #13791

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Apr 24, 2017
Merged

Prefer legacy kinds #13791

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5dd6cae
Prefer legacy kinds
ncdc Apr 17, 2017
164216c
Check multiple GVKs in AddObjectsToTemplate
ncdc Apr 18, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/authorization/api/install/apigroup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
"github.com/openshift/origin/pkg/authorization/api/v1"
)

func init() {
func installApiGroup() {
if err := announced.NewGroupMetaFactory(
&announced.GroupMetaFactoryArgs{
GroupName: api.GroupName,
Expand Down
2 changes: 2 additions & 0 deletions pkg/authorization/api/install/install.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ func init() {
if err := enableVersions(externalVersions); err != nil {
panic(err)
}

installApiGroup()
}

// TODO: enableVersions should be centralized rather than spread in each API
Expand Down
2 changes: 1 addition & 1 deletion pkg/build/api/install/apigroup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"github.com/openshift/origin/pkg/build/api/v1"
)

func init() {
func installApiGroup() {
if err := announced.NewGroupMetaFactory(
&announced.GroupMetaFactoryArgs{
GroupName: api.GroupName,
Expand Down
2 changes: 2 additions & 0 deletions pkg/build/api/install/install.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ func init() {
if err := enableVersions(externalVersions); err != nil {
panic(err)
}

installApiGroup()
}

// TODO: enableVersions should be centralized rather than spread in each API
Expand Down
2 changes: 1 addition & 1 deletion pkg/deploy/api/install/apigroup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"github.com/openshift/origin/pkg/deploy/api/v1"
)

func init() {
func installApiGroup() {
if err := announced.NewGroupMetaFactory(
&announced.GroupMetaFactoryArgs{
GroupName: api.GroupName,
Expand Down
2 changes: 2 additions & 0 deletions pkg/deploy/api/install/install.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ func init() {
if err := enableVersions(externalVersions); err != nil {
panic(err)
}

installApiGroup()
}

// TODO: enableVersions should be centralized rather than spread in each API
Expand Down
2 changes: 1 addition & 1 deletion pkg/image/api/install/apigroup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
"github.com/openshift/origin/pkg/image/api/v1"
)

func init() {
func installApiGroup() {
if err := announced.NewGroupMetaFactory(
&announced.GroupMetaFactoryArgs{
GroupName: api.GroupName,
Expand Down
2 changes: 2 additions & 0 deletions pkg/image/api/install/install.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ func init() {
if err := enableVersions(externalVersions); err != nil {
panic(err)
}

installApiGroup()
}

// TODO: enableVersions should be centralized rather than spread in each API
Expand Down
2 changes: 1 addition & 1 deletion pkg/oauth/api/install/apigroup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
"github.com/openshift/origin/pkg/oauth/api/v1"
)

func init() {
func installApiGroup() {
if err := announced.NewGroupMetaFactory(
&announced.GroupMetaFactoryArgs{
GroupName: api.GroupName,
Expand Down
2 changes: 2 additions & 0 deletions pkg/oauth/api/install/install.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ func init() {
if err := enableVersions(externalVersions); err != nil {
panic(err)
}

installApiGroup()
}

// TODO: enableVersions should be centralized rather than spread in each API
Expand Down
2 changes: 1 addition & 1 deletion pkg/project/api/install/apigroup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
"github.com/openshift/origin/pkg/project/api/v1"
)

func init() {
func installApiGroup() {
if err := announced.NewGroupMetaFactory(
&announced.GroupMetaFactoryArgs{
GroupName: api.GroupName,
Expand Down
2 changes: 2 additions & 0 deletions pkg/project/api/install/install.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ func init() {
if err := enableVersions(externalVersions); err != nil {
panic(err)
}

installApiGroup()
}

// TODO: enableVersions should be centralized rather than spread in each API
Expand Down
2 changes: 1 addition & 1 deletion pkg/quota/api/install/apigroup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
"github.com/openshift/origin/pkg/quota/api/v1"
)

func init() {
func installApiGroup() {
if err := announced.NewGroupMetaFactory(
&announced.GroupMetaFactoryArgs{
GroupName: api.GroupName,
Expand Down
2 changes: 2 additions & 0 deletions pkg/quota/api/install/install.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ func init() {
glog.V(4).Infof("%v", err)
return
}

installApiGroup()
}

// TODO: enableVersions should be centralized rather than spread in each API
Expand Down
2 changes: 1 addition & 1 deletion pkg/route/api/install/apigroup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"github.com/openshift/origin/pkg/route/api/v1"
)

func init() {
func installApiGroup() {
if err := announced.NewGroupMetaFactory(
&announced.GroupMetaFactoryArgs{
GroupName: api.GroupName,
Expand Down
2 changes: 2 additions & 0 deletions pkg/route/api/install/install.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ func init() {
if err := enableVersions(externalVersions); err != nil {
panic(err)
}

installApiGroup()
}

// TODO: enableVersions should be centralized rather than spread in each API
Expand Down
2 changes: 1 addition & 1 deletion pkg/sdn/api/install/apigroup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
"github.com/openshift/origin/pkg/sdn/api/v1"
)

func init() {
func installApiGroup() {
if err := announced.NewGroupMetaFactory(
&announced.GroupMetaFactoryArgs{
GroupName: api.GroupName,
Expand Down
2 changes: 2 additions & 0 deletions pkg/sdn/api/install/install.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ func init() {
if err := enableVersions(externalVersions); err != nil {
panic(err)
}

installApiGroup()
}

// TODO: enableVersions should be centralized rather than spread in each API
Expand Down
2 changes: 1 addition & 1 deletion pkg/security/api/install/apigroup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"github.com/openshift/origin/pkg/security/api/v1"
)

func init() {
func installApiGroup() {
if err := announced.NewGroupMetaFactory(
&announced.GroupMetaFactoryArgs{
GroupName: api.GroupName,
Expand Down
2 changes: 2 additions & 0 deletions pkg/security/api/install/install.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ func init() {
if err := enableVersions(externalVersions); err != nil {
panic(err)
}

installApiGroup()
}

// TODO: enableVersions should be centralized rather than spread in each API
Expand Down
16 changes: 11 additions & 5 deletions pkg/template/api/helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,27 @@ func AddObjectsToTemplate(template *Template, objects []runtime.Object, targetVe
return errors.New("cannot add a nil object to a template")
}

kind, _, err := kapi.Scheme.ObjectKind(obj)
// We currently add legacy types first to the scheme, followed by the types in the new api
// groups. We have to check all ObjectKinds and not just use the first one returned by
// ObjectKind().
gvks, _, err := kapi.Scheme.ObjectKinds(obj)
if err != nil {
return err
}

var targetVersion *unversioned.GroupVersion
Copy link
Contributor

Choose a reason for hiding this comment

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

This logic is really confused at this point - Codecs.CodecForVersions accepts a runtime.GroupVersioner (which gvks satisfies) which should be enough to ensure you get the right object, so you shouldn't have to pass a single target version (you can instead pass multiple). And it raises the question about whether we should even be using this logic for templates vs dealing with unstructured.

However, I guess it can stand for now.

outerLoop:
for j := range targetVersions {
possibleVersion := targetVersions[j]
if kind.Group == possibleVersion.Group {
targetVersion = &possibleVersion
break
for _, kind := range gvks {
if kind.Group == possibleVersion.Group {
targetVersion = &possibleVersion
break outerLoop
}
}
}
if targetVersion == nil {
return fmt.Errorf("no target version found for object[%d], kind %v in %v", i, kind, targetVersions)
return fmt.Errorf("no target version found for object[%d], gvks %v in %v", i, gvks, targetVersions)
}

wrappedObject := runtime.NewEncodable(kapi.Codecs.LegacyCodec(*targetVersion), obj)
Expand Down
2 changes: 1 addition & 1 deletion pkg/template/api/install/apigroup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
"github.com/openshift/origin/pkg/template/api/v1"
)

func init() {
func installApiGroup() {
if err := announced.NewGroupMetaFactory(
&announced.GroupMetaFactoryArgs{
GroupName: api.GroupName,
Expand Down
2 changes: 2 additions & 0 deletions pkg/template/api/install/install.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ func init() {
if err := enableVersions(externalVersions); err != nil {
panic(err)
}

installApiGroup()
}

// TODO: enableVersions should be centralized rather than spread in each API
Expand Down
2 changes: 1 addition & 1 deletion pkg/user/api/install/apigroup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
"github.com/openshift/origin/pkg/user/api/v1"
)

func init() {
func installApiGroup() {
if err := announced.NewGroupMetaFactory(
&announced.GroupMetaFactoryArgs{
GroupName: api.GroupName,
Expand Down
2 changes: 2 additions & 0 deletions pkg/user/api/install/install.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ func init() {
if err := enableVersions(externalVersions); err != nil {
panic(err)
}

installApiGroup()
}

// TODO: enableVersions should be centralized rather than spread in each API
Expand Down
4 changes: 2 additions & 2 deletions test/cmd/newapp.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ os::cmd::expect_success 'oc new-app -f test/testdata/template-with-namespaces.js
os::cmd::expect_success 'oc delete all -l app=ruby-helloworld-sample'

# ensure non-duplicate invalid label errors show up
os::cmd::expect_failure_and_text 'oc new-app nginx -l qwer1345%$$#=self' 'error: ImageStream.image.openshift.io "nginx" is invalid'
os::cmd::expect_failure_and_text 'oc new-app nginx -l qwer1345%$$#=self' 'DeploymentConfig.apps.openshift.io "nginx" is invalid'
os::cmd::expect_failure_and_text 'oc new-app nginx -l qwer1345%$$#=self' 'error: ImageStream "nginx" is invalid'
os::cmd::expect_failure_and_text 'oc new-app nginx -l qwer1345%$$#=self' 'DeploymentConfig "nginx" is invalid'
os::cmd::expect_failure_and_text 'oc new-app nginx -l qwer1345%$$#=self' 'Service "nginx" is invalid'

# check if we can create from a stored template
Expand Down