fix up image policy admission plugin #10384

deads2k · 2016-08-12T16:13:35Z

Found these while trying to write up documentation and examples for the ImagePolicy admission plugin.

I think we should move resolve to a top level element that is an enum

RequiredRewrite - require resolution to succeed and rewrite the resource to use it
Required - require resolution to succeed, but don't rewrite the image pull spec
AttemptRewrite - attempt resolution, rewrite if successful
Attempt - attempt resolution, don't rewrite
DoNotAttempt

Individual rules could have a SkipOnResolutionFailure which would allow a best effor attempt. It would be a validation error to choose DoNotAttempt with a policy rule that requires it.

This would remove some of the craziness I faced when trying to match a registry name and having to say that I tolerated resolution failures.

@smarterclayton

deads2k · 2016-08-12T17:25:29Z

[test]

smarterclayton · 2016-08-12T17:27:25Z

SGTM API approved - will not have a chance to review before EOD.

deads2k · 2016-08-15T19:06:09Z

@mfojtik We need this in 3.3 ptal or reassign.

mfojtik · 2016-08-15T19:18:56Z

pkg/image/admission/imagepolicy/accept.go

+				// an objectref that is DockerImage ref will have a name that corresponds to its pull spec.  We can parse that
+				// to a docker image ref
+				if ref != nil && ref.Kind == "DockerImage" {
+					attrs.Name, _ = imageapi.ParseDockerImageReference(ref.Name)


why dropping the errors?

why dropping the errors?

We won't return the error (resolution is optional) and if it doesn't understand it, it returns the best parse it can.

mfojtik · 2016-08-15T19:21:18Z

LGTM (do we have test coverage for the DockerImage case?)

mfojtik · 2016-08-16T08:02:04Z

[merge]

openshift-bot · 2016-08-16T09:32:23Z

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/7959/) (Image: devenv-rhel7_4839)

deads2k · 2016-08-16T12:17:07Z

#10002 https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_conformance/5138/testReport/junit/(root)/Extended/deploymentconfigs_with_test_deployments_should_run_a_deployment_to_completion_and_then_scale_to_zero__Conformance_/ re[test] re[merge]

openshift-bot · 2016-08-16T12:17:23Z

Evaluated for origin test up to a40658d

openshift-bot · 2016-08-16T12:17:33Z

Evaluated for origin merge up to a40658d

openshift-bot · 2016-08-16T13:38:52Z

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/7958/)

0xmichalis · 2016-08-17T10:25:48Z

#10002 https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_conformance/5138/testReport/junit/(root)/Extended/deploymentconfigs_with_test_deployments_should_run_a_deployment_to_completion_and_then_scale_to_zero__Conformance_/

The failure you stumbled upon is different from the one in the linked issue. What you saw is that the deployer pod stopped at some point:

--> pre: Success
--> Scaling up deployment-test-3 from 0 to 1, scaling down deployment-test-2 from 0 to 0 (keep 1 pods available, don't exceed 2 pods)
    Scaling deployment-test-3 up to 1
Aug 16 05:03:25.949: INFO: DC: &api.DeploymentConfig{TypeMeta:unversioned.TypeMeta{Kind:"", APIVersion:""}, ObjectMeta:api.ObjectMeta{Name:"deployment-test", GenerateName:"", Namespace:"extended-test-cli-deployment-135yw-60mtz", SelfLink:"/oapi/v1/namespaces/extended-test-cli-deployment-135yw-60mtz/deploymentconfigs/deployment-test", UID:"2b860b18-6390-11e6-a59b-0e3a02572a5f", ResourceVersion:"10970", Generation:4, CreationTimestamp:unversioned.Time{Time:time.Time{sec:63606934953, nsec:0, loc:(*time.Location)(0x67640e0)}}, DeletionTimestamp:(*unversioned.Time)(nil), DeletionGracePeriodSeconds:(*int64)(nil), Labels:map[string]string(nil), Annotations:map[string]string(nil), OwnerReferences:[]api.OwnerReference(nil), Finalizers:[]string(nil)}, Spec:api.DeploymentConfigSpec{Strategy:api.DeploymentStrategy{Type:"Rolling", RecreateParams:(*api.RecreateDeploymentStrategyParams)(nil), RollingParams:(*api.RollingDeploymentStrategyParams)(0xc82059e0e0), CustomParams:(*api.CustomDeploymentStrategyParams)(nil), Resources:api.ResourceRequirements{Limits:api.ResourceList(nil), Requests:api.ResourceList(nil)}, Labels:map[string]string(nil), Annotations:map[string]string(nil)}, MinReadySeconds:0, Triggers:[]api.DeploymentTriggerPolicy{api.DeploymentTriggerPolicy{Type:"ConfigChange", ImageChangeParams:(*api.DeploymentTriggerImageChangeParams)(nil)}}, Replicas:1, RevisionHistoryLimit:(*int32)(nil), Test:true, Paused:false, Selector:map[string]string{"name":"deployment-test"}, Template:(*api.PodTemplateSpec)(0xc82190aa80)}, Status:api.DeploymentConfigStatus{LatestVersion:3, ObservedGeneration:4, Replicas:0, UpdatedReplicas:0, AvailableReplicas:0, UnavailableReplicas:0, Details:(*api.DeploymentDetails)(0xc820d446f0)}}
Aug 16 05:03:25.949: INFO:   RCs: []api.ReplicationController{api.ReplicationController{TypeMeta:unversioned.TypeMeta{Kind:"", APIVersion:""}, ObjectMeta:api.ObjectMeta{Name:"deployment-test-1", GenerateName:"", Namespace:"extended-test-cli-deployment-135yw-60mtz", SelfLink:"/api/v1/namespaces/extended-test-cli-deployment-135yw-60mtz/replicationcontrollers/deployment-test-1", UID:"2b8aa861-6390-11e6-a59b-0e3a02572a5f", ResourceVersion:"10843", Generation:3, CreationTimestamp:unversioned.Time{Time:time.Time{sec:63606934953, nsec:0, loc:(*time.Location)(0x67640e0)}}, DeletionTimestamp:(*unversioned.Time)(nil), DeletionGracePeriodSeconds:(*int64)(nil), Labels:map[string]string{"openshift.io/deployment-config.name":"deployment-test"}, Annotations:map[string]string{"openshift.io/deployment.replicas":"0", "openshift.io/deployment.status-reason":"caused by a config change", "openshift.io/encoded-deployment-config":"{\"kind\":\"DeploymentConfig\",\"apiVersion\":\"v1\",\"metadata\":{\"name\":\"deployment-test\",\"namespace\":\"extended-test-cli-deployment-135yw-60mtz\",\"selfLink\":\"/oapi/v1/namespaces/extended-test-cli-deployment-135yw-60mtz/deploymentconfigs/deployment-test\",\"uid\":\"2b860b18-6390-11e6-a59b-0e3a02572a5f\",\"resourceVersion\":\"10734\",\"generation\":1,\"creationTimestamp\":\"2016-08-16T09:02:33Z\"},\"spec\":{\"strategy\":{\"type\":\"Rolling\",\"rollingParams\":{\"updatePeriodSeconds\":1,\"intervalSeconds\":1,\"timeoutSeconds\":600,\"maxUnavailable\":\"25%\",\"maxSurge\":\"25%\",\"pre\":{\"failurePolicy\":\"Abort\",\"execNewPod\":{\"command\":[\"/bin/echo\",\"test pre hook executed\"],\"containerName\":\"myapp\"}}},\"resources\":{}},\"triggers\":[{\"type\":\"ConfigChange\"}],\"replicas\":2,\"test\":true,\"selector\":{\"name\":\"deployment-test\"},\"template\":{\"metadata\":{\"creationTimestamp\":null,\"labels\":{\"name\":\"deployment-test\"}},\"spec\":{\"containers\":[{\"name\":\"myapp\",\"image\":\"docker.io/centos:centos7\",\"command\":[\"/bin/sleep\",\"100\"],\"resources\":{},\"terminationMessagePath\":\"/dev/termination-log\",\"imagePullPolicy\":\"IfNotPresent\"}],\"restartPolicy\":\"Always\",\"terminationGracePeriodSeconds\":0,\"dnsPolicy\":\"ClusterFirst\",\"securityContext\":{}}}},\"status\":{\"latestVersion\":1,\"details\":{\"message\":\"caused by a config change\",\"causes\":[{\"type\":\"ConfigChange\"}]}}}\n", "kubectl.kubernetes.io/original-replicas":"0", "openshift.io/deployer-pod.name":"deployment-test-1-deploy", "openshift.io/deployment-config.latest-version":"1", "openshift.io/deployment-config.name":"deployment-test", "openshift.io/deployment.phase":"Complete"}, OwnerReferences:[]api.OwnerReference(nil), Finalizers:[]string(nil)}, Spec:api.ReplicationControllerSpec{Replicas:0, Selector:map[string]string{"name":"deployment-test", "deployment":"deployment-test-1", "deploymentconfig":"deployment-test"}, Template:(*api.PodTemplateSpec)(0xc82190ac40)}, Status:api.ReplicationControllerStatus{Replicas:0, FullyLabeledReplicas:0, ObservedGeneration:3}}, api.ReplicationController{TypeMeta:unversioned.TypeMeta{Kind:"", APIVersion:""}, ObjectMeta:api.ObjectMeta{Name:"deployment-test-2", GenerateName:"", Namespace:"extended-test-cli-deployment-135yw-60mtz", SelfLink:"/api/v1/namespaces/extended-test-cli-deployment-135yw-60mtz/replicationcontrollers/deployment-test-2", UID:"3a6be645-6390-11e6-a59b-0e3a02572a5f", ResourceVersion:"10908", Generation:3, CreationTimestamp:unversioned.Time{Time:time.Time{sec:63606934978, nsec:0, loc:(*time.Location)(0x67640e0)}}, DeletionTimestamp:(*unversioned.Time)(nil), DeletionGracePeriodSeconds:(*int64)(nil), Labels:map[string]string{"openshift.io/deployment-config.name":"deployment-test"}, Annotations:map[string]string{"kubectl.kubernetes.io/original-replicas":"0", "openshift.io/deployer-pod.name":"deployment-test-2-deploy", "openshift.io/deployment-config.latest-version":"2", "openshift.io/deployment-config.name":"deployment-test", "openshift.io/deployment.phase":"Complete", "openshift.io/deployment.replicas":"0", "openshift.io/deployment.status-reason":"caused by a config change", "openshift.io/encoded-deployment-config":"{\"kind\":\"DeploymentConfig\",\"apiVersion\":\"v1\",\"metadata\":{\"name\":\"deployment-test\",\"namespace\":\"extended-test-cli-deployment-135yw-60mtz\",\"selfLink\":\"/oapi/v1/namespaces/extended-test-cli-deployment-135yw-60mtz/deploymentconfigs/deployment-test\",\"uid\":\"2b860b18-6390-11e6-a59b-0e3a02572a5f\",\"resourceVersion\":\"10812\",\"generation\":3,\"creationTimestamp\":\"2016-08-16T09:02:33Z\"},\"spec\":{\"strategy\":{\"type\":\"Rolling\",\"rollingParams\":{\"updatePeriodSeconds\":1,\"intervalSeconds\":1,\"timeoutSeconds\":600,\"maxUnavailable\":\"25%\",\"maxSurge\":\"25%\",\"pre\":{\"failurePolicy\":\"Abort\",\"execNewPod\":{\"command\":[\"/bin/echo\",\"test pre hook executed\"],\"containerName\":\"myapp\"}}},\"resources\":{}},\"triggers\":[{\"type\":\"ConfigChange\"}],\"replicas\":1,\"test\":true,\"selector\":{\"name\":\"deployment-test\"},\"template\":{\"metadata\":{\"creationTimestamp\":null,\"labels\":{\"name\":\"deployment-test\"}},\"spec\":{\"containers\":[{\"name\":\"myapp\",\"image\":\"docker.io/centos:centos7\",\"command\":[\"/bin/sleep\",\"100\"],\"resources\":{},\"terminationMessagePath\":\"/dev/termination-log\",\"imagePullPolicy\":\"IfNotPresent\"}],\"restartPolicy\":\"Always\",\"terminationGracePeriodSeconds\":0,\"dnsPolicy\":\"ClusterFirst\",\"securityContext\":{}}}},\"status\":{\"latestVersion\":2,\"observedGeneration\":2,\"details\":{\"message\":\"caused by a config change\",\"causes\":[{\"type\":\"ConfigChange\"}]}}}\n"}, OwnerReferences:[]api.OwnerReference(nil), Finalizers:[]string(nil)}, Spec:api.ReplicationControllerSpec{Replicas:0, Selector:map[string]string{"deployment":"deployment-test-2", "deploymentconfig":"deployment-test", "name":"deployment-test"}, Template:(*api.PodTemplateSpec)(0xc82190ae00)}, Status:api.ReplicationControllerStatus{Replicas:0, FullyLabeledReplicas:0, ObservedGeneration:3}}, api.ReplicationController{TypeMeta:unversioned.TypeMeta{Kind:"", APIVersion:""}, ObjectMeta:api.ObjectMeta{Name:"deployment-test-3", GenerateName:"", Namespace:"extended-test-cli-deployment-135yw-60mtz", SelfLink:"/api/v1/namespaces/extended-test-cli-deployment-135yw-60mtz/replicationcontrollers/deployment-test-3", UID:"424d5c31-6390-11e6-a59b-0e3a02572a5f", ResourceVersion:"10969", Generation:3, CreationTimestamp:unversioned.Time{Time:time.Time{sec:63606934992, nsec:0, loc:(*time.Location)(0x67640e0)}}, DeletionTimestamp:(*unversioned.Time)(nil), DeletionGracePeriodSeconds:(*int64)(nil), Labels:map[string]string{"openshift.io/deployment-config.name":"deployment-test"}, Annotations:map[string]string{"openshift.io/deployment.phase":"Complete", "openshift.io/deployment.replicas":"0", "openshift.io/deployment.status-reason":"caused by a config change", "openshift.io/encoded-deployment-config":"{\"kind\":\"DeploymentConfig\",\"apiVersion\":\"v1\",\"metadata\":{\"name\":\"deployment-test\",\"namespace\":\"extended-test-cli-deployment-135yw-60mtz\",\"selfLink\":\"/oapi/v1/namespaces/extended-test-cli-deployment-135yw-60mtz/deploymentconfigs/deployment-test\",\"uid\":\"2b860b18-6390-11e6-a59b-0e3a02572a5f\",\"resourceVersion\":\"10876\",\"generation\":4,\"creationTimestamp\":\"2016-08-16T09:02:33Z\"},\"spec\":{\"strategy\":{\"type\":\"Rolling\",\"rollingParams\":{\"updatePeriodSeconds\":1,\"intervalSeconds\":1,\"timeoutSeconds\":600,\"maxUnavailable\":\"25%\",\"maxSurge\":\"25%\",\"pre\":{\"failurePolicy\":\"Abort\",\"execNewPod\":{\"command\":[\"/bin/echo\",\"test pre hook executed\"],\"containerName\":\"myapp\"}}},\"resources\":{}},\"triggers\":[{\"type\":\"ConfigChange\"}],\"replicas\":1,\"test\":true,\"selector\":{\"name\":\"deployment-test\"},\"template\":{\"metadata\":{\"creationTimestamp\":null,\"labels\":{\"name\":\"deployment-test\"}},\"spec\":{\"containers\":[{\"name\":\"myapp\",\"image\":\"docker.io/centos:centos7\",\"command\":[\"/bin/sleep\",\"100\"],\"resources\":{},\"terminationMessagePath\":\"/dev/termination-log\",\"imagePullPolicy\":\"IfNotPresent\"}],\"restartPolicy\":\"Always\",\"terminationGracePeriodSeconds\":0,\"dnsPolicy\":\"ClusterFirst\",\"securityContext\":{}}}},\"status\":{\"latestVersion\":3,\"observedGeneration\":3,\"details\":{\"message\":\"caused by a config change\",\"causes\":[{\"type\":\"ConfigChange\"}]}}}\n", "openshift.io/deployer-pod.name":"deployment-test-3-deploy", "openshift.io/deployment-config.latest-version":"3", "openshift.io/deployment-config.name":"deployment-test"}, OwnerReferences:[]api.OwnerReference(nil), Finalizers:[]string(nil)}, Spec:api.ReplicationControllerSpec{Replicas:0, Selector:map[string]string{"deployment":"deployment-test-3", "deploymentconfig":"deployment-test", "name":"deployment-test"}, Template:(*api.PodTemplateSpec)(0xc82190afc0)}, Status:api.ReplicationControllerStatus{Replicas:0, FullyLabeledReplicas:0, ObservedGeneration:3}}}

• Failure [57.566 seconds]
deploymentconfigs
/data/src/github.com/openshift/origin/test/extended/deployments/deployments.go:629
  with test deployments
  /data/src/github.com/openshift/origin/test/extended/deployments/deployments.go:243
    should run a deployment to completion and then scale to zero [Conformance] [It]
    /data/src/github.com/openshift/origin/test/extended/deployments/deployments.go:242

    Expected
        <string>: Started deployment #3
        --> pre: Running hook pod ...
        test pre hook executed
        --> pre: Success
        --> Scaling up deployment-test-3 from 0 to 1, scaling down deployment-test-2 from 0 to 0 (keep 1 pods available, don't exceed 2 pods)
            Scaling deployment-test-3 up to 1
    to contain substring
        <string>: --> Success

    /data/src/github.com/openshift/origin/test/extended/deployments/deployments.go:240
------------------------------

@mfojtik @smarterclayton not sure why we couldn't get all the logs from the deployer. It seems that the third deployment is completed successfully (and the deployer pod is deleted).

api.ReplicationController{TypeMeta:unversioned.TypeMeta{Kind:"", APIVersion:""}, ObjectMeta:api.ObjectMeta{Name:"deployment-test-3", GenerateName:"", Namespace:"extended-test-cli-deployment-135yw-60mtz", SelfLink:"/api/v1/namespaces/extended-test-cli-deployment-135yw-60mtz/replicationcontrollers/deployment-test-3", UID:"424d5c31-6390-11e6-a59b-0e3a02572a5f", ResourceVersion:"10969", Generation:3, CreationTimestamp:unversioned.Time{Time:time.Time{sec:63606934992, nsec:0, loc:(*time.Location)(0x67640e0)}}, DeletionTimestamp:(*unversioned.Time)(nil), DeletionGracePeriodSeconds:(*int64)(nil), Labels:map[string]string{"openshift.io/deployment-config.name":"deployment-test"}, Annotations:map[string]string{"openshift.io/deployment.phase":"Complete",

Is it possible that the deployer pod is deleted faster than reading all its log?

fix up image policy admission plugin

a40658d

smarterclayton added the api-approved label Aug 12, 2016

deads2k assigned mfojtik Aug 15, 2016

deads2k added this to the 1.3.0 milestone Aug 15, 2016

mfojtik reviewed Aug 15, 2016
View reviewed changes

openshift-bot merged commit 97d149e into openshift:master Aug 16, 2016

deads2k deleted the fixup-image branch September 6, 2016 17:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix up image policy admission plugin #10384

fix up image policy admission plugin #10384

deads2k commented Aug 12, 2016

deads2k commented Aug 12, 2016

smarterclayton commented Aug 12, 2016

deads2k commented Aug 15, 2016

mfojtik Aug 15, 2016

deads2k Aug 15, 2016

mfojtik commented Aug 15, 2016

mfojtik commented Aug 16, 2016

openshift-bot commented Aug 16, 2016 •

edited

Loading

deads2k commented Aug 16, 2016

openshift-bot commented Aug 16, 2016

openshift-bot commented Aug 16, 2016

openshift-bot commented Aug 16, 2016

0xmichalis commented Aug 17, 2016

fix up image policy admission plugin #10384

fix up image policy admission plugin #10384

Conversation

deads2k commented Aug 12, 2016

deads2k commented Aug 12, 2016

smarterclayton commented Aug 12, 2016

deads2k commented Aug 15, 2016

mfojtik Aug 15, 2016

Choose a reason for hiding this comment

deads2k Aug 15, 2016

Choose a reason for hiding this comment

mfojtik commented Aug 15, 2016

mfojtik commented Aug 16, 2016

openshift-bot commented Aug 16, 2016 • edited Loading

deads2k commented Aug 16, 2016

openshift-bot commented Aug 16, 2016

openshift-bot commented Aug 16, 2016

openshift-bot commented Aug 16, 2016

0xmichalis commented Aug 17, 2016

openshift-bot commented Aug 16, 2016 •

edited

Loading