Skip to content
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
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
198 changes: 23 additions & 175 deletions hack/openapi-violation.list

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions hack/update-generated-conversions.sh
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ ALL_FQ_APIS=(
github.com/openshift/openshift-apiserver/pkg/authorization/apis/authorization/v1
github.com/openshift/openshift-apiserver/pkg/build/apis/build/v1
github.com/openshift/openshift-apiserver/pkg/image/apis/image/v1
github.com/openshift/openshift-apiserver/pkg/image/apis/image/dockerpre012
github.com/openshift/openshift-apiserver/pkg/image/apis/image/docker10
github.com/openshift/openshift-apiserver/pkg/oauth/apis/oauth/v1
github.com/openshift/openshift-apiserver/pkg/project/apis/project/v1
github.com/openshift/openshift-apiserver/pkg/project/apiserver/admission/apis/requestlimit/v1
Expand Down
3 changes: 2 additions & 1 deletion pkg/api/compatibility_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ func TestCompatibility_v1_Pod(t *testing.T) {
testCompatibility(
t, "v1", input,
func(obj runtime.Object) field.ErrorList {
return validation.ValidatePod(obj.(*api.Pod), validation.PodValidationOptions{})
pod := obj.(*api.Pod)
return validation.ValidatePodSpec(&pod.Spec, &pod.ObjectMeta, field.NewPath("spec"))
},
map[string]string{
"spec.serviceAccount": expectedServiceAccount,
Expand Down
4 changes: 4 additions & 0 deletions pkg/api/legacy/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import (
imagev1 "github.com/openshift/api/image/v1"
"github.com/openshift/openshift-apiserver/pkg/api/apihelpers"
"github.com/openshift/openshift-apiserver/pkg/image/apis/image"
imagedocker10helpers "github.com/openshift/openshift-apiserver/pkg/image/apis/image/docker10"
imagedockerpre012helpers "github.com/openshift/openshift-apiserver/pkg/image/apis/image/dockerpre012"
imagev1helpers "github.com/openshift/openshift-apiserver/pkg/image/apis/image/v1"
)

Expand All @@ -28,6 +30,8 @@ func InstallInternalLegacyImage(scheme *runtime.Scheme) {
addLegacyImageFieldSelectorKeyConversions,
imagev1helpers.RegisterDefaults,
imagev1helpers.RegisterConversions,
imagedockerpre012helpers.RegisterConversions,
imagedocker10helpers.RegisterConversions,
)
utilruntime.Must(schemeBuilder.AddToScheme(scheme))
}
Expand Down
14 changes: 13 additions & 1 deletion pkg/api/validation/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,16 +232,28 @@ func TestPodSpecNodeSelectorUpdateDisallowed(t *testing.T) {
oldPod := &kapi.Pod{
ObjectMeta: metav1.ObjectMeta{
ResourceVersion: "1",
Name: "test",
Namespace: "test",
},
Spec: kapi.PodSpec{
Containers: []kapi.Container{
{
Name: "test",
Image: "test",
TerminationMessagePolicy: kapi.TerminationMessageFallbackToLogsOnError,
ImagePullPolicy: kapi.PullAlways,
},
},
RestartPolicy: kapi.RestartPolicyAlways,
DNSPolicy: kapi.DNSClusterFirst,
NodeSelector: map[string]string{
"foo": "bar",
},
},
}

if errs := validation.ValidatePodUpdate(oldPod, oldPod, validation.PodValidationOptions{}); len(errs) != 0 {
t.Fatal("expected no errors")
t.Fatalf("expected no errors, got: %+v", errs)
}

newPod := *oldPod
Expand Down
24 changes: 14 additions & 10 deletions pkg/apps/apis/apps/v1/conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package v1
import (
"strings"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/conversion"
"k8s.io/apimachinery/pkg/util/intstr"

Expand Down Expand Up @@ -45,7 +46,7 @@ func Convert_apps_DeploymentTriggerImageChangeParams_To_v1_DeploymentTriggerImag
return nil
}

func Convert_v1_RollingDeploymentStrategyParams_To_apps_RollingDeploymentStrategyParams(in *v1.RollingDeploymentStrategyParams, out *newer.RollingDeploymentStrategyParams, s conversion.Scope) error {
func Convert_v1_RollingDeploymentStrategyParams_To_apps_RollingDeploymentStrategyParams(in *v1.RollingDeploymentStrategyParams, out *newer.RollingDeploymentStrategyParams, _ conversion.Scope) error {
SetDefaults_RollingDeploymentStrategyParams(in)

out.UpdatePeriodSeconds = in.UpdatePeriodSeconds
Expand All @@ -54,43 +55,44 @@ func Convert_v1_RollingDeploymentStrategyParams_To_apps_RollingDeploymentStrateg

if in.Pre != nil {
out.Pre = &newer.LifecycleHook{}
if err := Convert_v1_LifecycleHook_To_apps_LifecycleHook(in.Pre, out.Pre, s); err != nil {
if err := Convert_v1_LifecycleHook_To_apps_LifecycleHook(in.Pre, out.Pre, nil); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

why this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will revert.

return err
}
}
if in.Post != nil {
out.Post = &newer.LifecycleHook{}
if err := Convert_v1_LifecycleHook_To_apps_LifecycleHook(in.Post, out.Post, s); err != nil {
if err := Convert_v1_LifecycleHook_To_apps_LifecycleHook(in.Post, out.Post, nil); err != nil {
Copy link
Contributor Author

@p0lyn0mial p0lyn0mial Sep 2, 2020

Choose a reason for hiding this comment

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

@sttts what about the others in this pkg? I think I'm going to revert them as well.

return err
}
}
if in.MaxUnavailable != nil {
if err := s.Convert(in.MaxUnavailable, &out.MaxUnavailable, 0); err != nil {
if err := metav1.Convert_intstr_IntOrString_To_intstr_IntOrString(in.MaxUnavailable, &out.MaxUnavailable, nil); err != nil {
return err
}
}
if in.MaxSurge != nil {
if err := s.Convert(in.MaxSurge, &out.MaxSurge, 0); err != nil {
if err := metav1.Convert_intstr_IntOrString_To_intstr_IntOrString(in.MaxSurge, &out.MaxSurge, nil); err != nil {
return err
}
}

return nil
}

func Convert_apps_RollingDeploymentStrategyParams_To_v1_RollingDeploymentStrategyParams(in *newer.RollingDeploymentStrategyParams, out *v1.RollingDeploymentStrategyParams, s conversion.Scope) error {
func Convert_apps_RollingDeploymentStrategyParams_To_v1_RollingDeploymentStrategyParams(in *newer.RollingDeploymentStrategyParams, out *v1.RollingDeploymentStrategyParams, _ conversion.Scope) error {
out.UpdatePeriodSeconds = in.UpdatePeriodSeconds
out.IntervalSeconds = in.IntervalSeconds
out.TimeoutSeconds = in.TimeoutSeconds

if in.Pre != nil {
out.Pre = &v1.LifecycleHook{}
if err := Convert_apps_LifecycleHook_To_v1_LifecycleHook(in.Pre, out.Pre, s); err != nil {
if err := Convert_apps_LifecycleHook_To_v1_LifecycleHook(in.Pre, out.Pre, nil); err != nil {
return err
}
}
if in.Post != nil {
out.Post = &v1.LifecycleHook{}
if err := Convert_apps_LifecycleHook_To_v1_LifecycleHook(in.Post, out.Post, s); err != nil {
if err := Convert_apps_LifecycleHook_To_v1_LifecycleHook(in.Post, out.Post, nil); err != nil {
return err
}
}
Expand All @@ -101,11 +103,13 @@ func Convert_apps_RollingDeploymentStrategyParams_To_v1_RollingDeploymentStrateg
if out.MaxSurge == nil {
out.MaxSurge = &intstr.IntOrString{}
}
if err := s.Convert(&in.MaxUnavailable, out.MaxUnavailable, 0); err != nil {

if err := metav1.Convert_intstr_IntOrString_To_intstr_IntOrString(&in.MaxUnavailable, out.MaxUnavailable, nil); err != nil {
return err
}
if err := s.Convert(&in.MaxSurge, out.MaxSurge, 0); err != nil {
if err := metav1.Convert_intstr_IntOrString_To_intstr_IntOrString(&in.MaxSurge, out.MaxSurge, nil); err != nil {
return err
}

return nil
}
7 changes: 7 additions & 0 deletions pkg/apps/apis/apps/v1/zz_generated.defaults.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

148 changes: 4 additions & 144 deletions pkg/apps/apiserver/registry/appstest/testutil.go
Original file line number Diff line number Diff line change
@@ -1,21 +1,14 @@
package appstest

import (
"testing"

appsv1 "github.com/openshift/api/apps/v1"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/kubernetes/pkg/api/legacyscheme"

appsv1 "github.com/openshift/api/apps/v1"
)

const (
ImageStreamName = "test-image-stream"
ImageID = "0000000000000000000000000000000000000000000000000000000000000001"
DockerImageReference = "registry:5000/openshift/test-image-stream@sha256:0000000000000000000000000000000000000000000000000000000000000001"
ImageStreamName = "test-image-stream"
)

func OkDeploymentConfig(version int64) *appsv1.DeploymentConfig {
Expand Down Expand Up @@ -49,31 +42,13 @@ func OkDeploymentConfigStatus(version int64) appsv1.DeploymentConfigStatus {
}
}

func OkImageChangeDetails() *appsv1.DeploymentDetails {
return &appsv1.DeploymentDetails{
Causes: []appsv1.DeploymentCause{{
Type: appsv1.DeploymentTriggerOnImageChange,
ImageTrigger: &appsv1.DeploymentCauseImageTrigger{
From: corev1.ObjectReference{
Name: ImageStreamName + ":latest",
Kind: "ImageStreamTag",
}}}}}
}

func OkConfigChangeDetails() *appsv1.DeploymentDetails {
return &appsv1.DeploymentDetails{
Causes: []appsv1.DeploymentCause{{
Type: appsv1.DeploymentTriggerOnConfigChange,
}}}
}

func OkStrategy() appsv1.DeploymentStrategy {
return appsv1.DeploymentStrategy{
Type: appsv1.DeploymentStrategyTypeRecreate,
Resources: corev1.ResourceRequirements{
Limits: corev1.ResourceList{
corev1.ResourceName(corev1.ResourceCPU): resource.MustParse("10"),
corev1.ResourceName(corev1.ResourceMemory): resource.MustParse("10G"),
corev1.ResourceCPU: resource.MustParse("10"),
corev1.ResourceMemory: resource.MustParse("10G"),
},
},
RecreateParams: &appsv1.RecreateDeploymentStrategyParams{
Expand All @@ -83,54 +58,11 @@ func OkStrategy() appsv1.DeploymentStrategy {
}
}

func OkCustomStrategy() appsv1.DeploymentStrategy {
return appsv1.DeploymentStrategy{
Type: appsv1.DeploymentStrategyTypeCustom,
CustomParams: OkCustomParams(),
Resources: corev1.ResourceRequirements{
Limits: corev1.ResourceList{
corev1.ResourceName(corev1.ResourceCPU): resource.MustParse("10"),
corev1.ResourceName(corev1.ResourceMemory): resource.MustParse("10G"),
},
},
}
}

func OkCustomParams() *appsv1.CustomDeploymentStrategyParams {
return &appsv1.CustomDeploymentStrategyParams{
Image: "openshift/origin-deployer",
Environment: []corev1.EnvVar{
{
Name: "ENV1",
Value: "VAL1",
},
},
Command: []string{"/bin/echo", "hello", "world"},
}
}

func mkintp(i int) *int64 {
v := int64(i)
return &v
}

func OkRollingStrategy() appsv1.DeploymentStrategy {
return appsv1.DeploymentStrategy{
Type: appsv1.DeploymentStrategyTypeRolling,
RollingParams: &appsv1.RollingDeploymentStrategyParams{
UpdatePeriodSeconds: mkintp(1),
IntervalSeconds: mkintp(1),
TimeoutSeconds: mkintp(20),
},
Resources: corev1.ResourceRequirements{
Limits: corev1.ResourceList{
corev1.ResourceName(corev1.ResourceCPU): resource.MustParse("10"),
corev1.ResourceName(corev1.ResourceMemory): resource.MustParse("10G"),
},
},
}
}

func OkSelector() map[string]string {
return map[string]string{"a": "b"}
}
Expand Down Expand Up @@ -173,24 +105,6 @@ func OkPodTemplate() *corev1.PodTemplateSpec {
}
}

func OkPodTemplateChanged() *corev1.PodTemplateSpec {
template := OkPodTemplate()
template.Spec.Containers[0].Image = DockerImageReference
return template
}

func OkPodTemplateMissingImage(missing ...string) *corev1.PodTemplateSpec {
set := sets.NewString(missing...)
template := OkPodTemplate()
for i, c := range template.Spec.Containers {
if set.Has(c.Name) {
// remember that slices use copies, so have to ref array entry explicitly
template.Spec.Containers[i].Image = ""
}
}
return template
}

func OkConfigChangeTrigger() appsv1.DeploymentTriggerPolicy {
return appsv1.DeploymentTriggerPolicy{
Type: appsv1.DeploymentTriggerOnConfigChange,
Expand All @@ -212,57 +126,3 @@ func OkImageChangeTrigger() appsv1.DeploymentTriggerPolicy {
},
}
}

func OkTriggeredImageChange() appsv1.DeploymentTriggerPolicy {
ict := OkImageChangeTrigger()
ict.ImageChangeParams.LastTriggeredImage = DockerImageReference
return ict
}

func OkNonAutomaticICT() appsv1.DeploymentTriggerPolicy {
ict := OkImageChangeTrigger()
ict.ImageChangeParams.Automatic = false
return ict
}

func OkTriggeredNonAutomatic() appsv1.DeploymentTriggerPolicy {
ict := OkNonAutomaticICT()
ict.ImageChangeParams.LastTriggeredImage = DockerImageReference
return ict
}

func TestDeploymentConfig(config *appsv1.DeploymentConfig) *appsv1.DeploymentConfig {
config.Spec.Test = true
return config
}

func RemoveTriggerTypes(config *appsv1.DeploymentConfig, triggerTypes ...appsv1.DeploymentTriggerType) {
types := sets.NewString()
for _, triggerType := range triggerTypes {
types.Insert(string(triggerType))
}

remaining := []appsv1.DeploymentTriggerPolicy{}
for _, trigger := range config.Spec.Triggers {
if types.Has(string(trigger.Type)) {
continue
}
remaining = append(remaining, trigger)
}

config.Spec.Triggers = remaining
}

func RoundTripConfig(t *testing.T, config *appsv1.DeploymentConfig) *appsv1.DeploymentConfig {
versioned, err := legacyscheme.Scheme.ConvertToVersion(config, appsv1.SchemeGroupVersion)
if err != nil {
t.Errorf("unexpected conversion error: %v", err)
return nil
}
defaulted, err := legacyscheme.Scheme.ConvertToVersion(versioned, appsv1.SchemeGroupVersion)
if err != nil {
t.Errorf("unexpected conversion error: %v", err)
return nil
}
return defaulted.(*appsv1.DeploymentConfig)
}
7 changes: 4 additions & 3 deletions pkg/apps/apiserver/registry/instantiate/rest.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"

v1 "github.com/openshift/openshift-apiserver/pkg/apps/apis/apps/v1"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
Expand Down Expand Up @@ -290,7 +291,7 @@ func canTrigger(

canTriggerByConfigChange := false
externalConfig := &appsv1.DeploymentConfig{}
if err := legacyscheme.Scheme.Convert(config, externalConfig, nil); err != nil {
if err := v1.Convert_apps_DeploymentConfig_To_v1_DeploymentConfig(config, externalConfig, nil); err != nil {
return false, nil, err
}
if appsutil.HasChangeTrigger(externalConfig) && // Our deployment config has a config change trigger
Expand All @@ -313,7 +314,7 @@ func decodeFromLatestDeployment(ctx context.Context, config *appsapi.DeploymentC
return config, nil
}
externalConfig := &appsv1.DeploymentConfig{}
if err := legacyscheme.Scheme.Convert(config, externalConfig, nil); err != nil {
if err := v1.Convert_apps_DeploymentConfig_To_v1_DeploymentConfig(config, externalConfig, nil); err != nil {
return nil, err
}
latestDeploymentName := appsutil.LatestDeploymentNameForConfig(externalConfig)
Expand All @@ -329,7 +330,7 @@ func decodeFromLatestDeployment(ctx context.Context, config *appsapi.DeploymentC
return nil, errors.NewInternalError(err)
}
internalConfig := &appsapi.DeploymentConfig{}
if err := legacyscheme.Scheme.Convert(decoded, internalConfig, nil); err != nil {
if err := v1.Convert_v1_DeploymentConfig_To_apps_DeploymentConfig(decoded, internalConfig, nil); err != nil {
return nil, err
}
return internalConfig, nil
Expand Down
Loading