add default for ProcMountType #69694

jessfraz · 2018-10-11T19:34:31Z

What this PR does / why we need it: This adds a default for the ProcMount feature and fixes #69647

kube-apiserver: fixes `procMount` field incorrectly being marked as required in openapi schema

jessfraz · 2018-10-11T19:34:46Z

we should also cherry-pick this on a release

dims · 2018-10-11T19:38:25Z

/assign @liggitt

Jordan, Fix for a backwards compatibility problem. Old yamls fail against 1.12.x

jessfraz · 2018-10-11T19:39:59Z

/priority critical-urgent

jessfraz · 2018-10-11T19:40:18Z

/kind bug

liggitt · 2018-10-11T23:22:42Z

The new field must be made optional/omitempty in types.go… setting a server side default won't fix the client side attempt to require the field in validation for old clients

liggitt · 2018-10-11T23:21:16Z

pkg/apis/core/v1/defaults.go

@@ -413,6 +416,13 @@ func SetDefaults_ScaleIOPersistentVolumeSource(obj *v1.ScaleIOPersistentVolumeSo
 	}
 }

+func SetDefaults_SecurityContext(obj *v1.SecurityContext) {
+	if obj.ProcMount == nil && utilfeature.DefaultFeatureGate.Enabled(features.ProcMountType) {


I didn't expect checking feature gates in defaulting… is this feature enabled by default?

Hmmm. I don't understand this either. I'm the original reporter (and a bit of a newb), but I don't believe this gate is enabled.

azureuser@k8s-master-71973844-0:~$ ps aux | grep feature-gate
root 9035 3.7 1.9 671344 155776 ? Ssl Oct10 54:10 /usr/local/bin/kubelet --enable-server --node-labels=kubernetes.io/role=master,kubernetes.azure.com/cluster=ClarityOrDie18RG --v=2 --volume-plugin-dir=/etc/kubernetes/volumeplugins --address=0.0.0.0 --allow-privileged=true --anonymous-auth=false --authorization-mode=Webhook --azure-container-registry-config=/etc/kubernetes/azure.json --cgroups-per-qos=true --client-ca-file=/etc/kubernetes/certs/ca.crt --cloud-config=/etc/kubernetes/azure.json --cloud-provider=azure --cluster-dns=10.0.0.10 --cluster-domain=cluster.local --enforce-node-allocatable=pods --event-qps=0 --eviction-hard=memory.available<100Mi,nodefs.available<10%,nodefs.inodesFree<5% --feature-gates=PodPriority=true --image-gc-high-threshold=85 --image-gc-low-threshold=80 --image-pull-progress-deadline=30m --keep-terminated-pod-volumes=false --kubeconfig=/var/lib/kubelet/kubeconfig --max-pods=30 --network-plugin=cni --node-status-update-frequency=10s --non-masquerade-cidr=10.240.0.0/12 --pod-infra-container-image=k8s.gcr.io/pause-amd64:3.1 --pod-manifest-path=/etc/kubernetes/manifests --pod-max-pids=100
root 11244 0.1 1.1 408860 92912 ? Ssl Oct10 2:49 /hyperkube proxy --kubeconfig=/var/lib/kubelet/kubeconfig --cluster-cidr=10.240.0.0/12 --feature-gates=ExperimentalCriticalPodAnnotation=true
root 18830 2.8 1.7 587656 145652 ? Ssl Oct11 6:29 /hyperkube controller-manager --allocate-node-cidrs=false --cloud-config=/etc/kubernetes/azure.json --cloud-provider=azure --cluster-cidr=10.240.0.0/12 --cluster-name=ClarityOrDie18 --cluster-signing-cert-file=/etc/kubernetes/certs/ca.crt --cluster-signing-key-file=/etc/kubernetes/certs/ca.key --configure-cloud-routes=false --feature-gates=ServiceNodeExclusion=true --kubeconfig=/var/lib/kubelet/kubeconfig --leader-elect=true --node-monitor-grace-period=40s --pod-eviction-timeout=5m0s --profiling=false --root-ca-file=/etc/kubernetes/certs/ca.crt --route-reconciliation-period=10s --service-account-private-key-file=/etc/kubernetes/certs/apiserver.key --terminated-pod-gc-threshold=5000 --use-service-account-credentials=true --v=2

it is not enabled by default, but there was already the feature gate check for a volume feature in that same file so I figured it was normal, if not i can remove

jessfraz · 2018-10-12T15:55:33Z

The new field must be made optional/omitempty in types.go… setting a server side default won't fix the client side attempt to require the field in validation for old clients

AFAICT it is set as optional am i missing something https://github.com/kubernetes/kubernetes/blob/master/pkg/apis/core/types.go#L4634

liggitt · 2018-10-12T16:04:12Z

kubernetes/staging/src/k8s.io/api/core/v1/types.go

Lines 5217 to 5222 in b5a5010

    
           // procMount denotes the type of proc mount to use for the containers. 
        
           // The default is DefaultProcMount which uses the container runtime defaults for 
        
           // readonly paths and masked paths. 
        
           // This requires the ProcMountType feature flag to be enabled. 
        
           // +optional 
        
           ProcMount *ProcMountType `json:"procMount,omitEmpty" protobuf:"bytes,9,opt,name=procMount"`

s/omitEmpty/omitempty/, I think. It still shows up as required in the openapi spec

Signed-off-by: Jess Frazelle <[email protected]>

liggitt · 2018-10-12T16:30:20Z

/lgtm
/approve

can you cherry-pick this to release-1.12 once tests are green?

k8s-ci-robot · 2018-10-12T16:30:26Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jessfraz, liggitt

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~api/OWNERS~~ [liggitt]
~~docs/api-reference/OWNERS~~ [liggitt]
~~staging/src/k8s.io/api/OWNERS~~ [liggitt]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

jessfraz · 2018-10-12T16:33:22Z

will do!

…

On Fri, Oct 12, 2018 at 9:32 AM Jordan Liggitt ***@***.***> wrote: /lgtm /approve can you cherry-pick this to release-1.12 once tests are green? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#69694 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABYNbJvhoZZWQlI_s7TjbI-D9rzMlLVqks5ukMQNgaJpZM4XYObz> .

-- Jessie Frazelle 4096R / D4C4 DD60 0D66 F65A 8EFC 511E 18F3 685C 0022 BFF3 pgp.mit.edu <http://pgp.mit.edu/pks/lookup?op=get&search=0x18F3685C0022BFF3>

jessfraz · 2018-10-12T16:45:29Z

/test pull-kubernetes-kubemark-e2e-gce-big

cherrypick for #69694

liggitt · 2018-10-15T15:54:52Z

added linting test for this in kubernetes/kube-openapi#106

jessfraz added this to the v1.12 milestone Oct 11, 2018

k8s-ci-robot requested review from deads2k and nikhiljindal October 11, 2018 19:35

k8s-ci-robot assigned liggitt Oct 11, 2018

k8s-ci-robot added the priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. label Oct 11, 2018

k8s-ci-robot added kind/bug Categorizes issue or PR as related to a bug. and removed needs-kind Indicates a PR lacks a `kind/foo` label and requires one. labels Oct 11, 2018

jessfraz force-pushed the 69647 branch from 7443998 to feb0c60 Compare October 11, 2018 20:14

k8s-ci-robot added area/kubelet sig/node Categorizes an issue or PR as relevant to SIG Node. labels Oct 11, 2018

liggitt reviewed Oct 11, 2018

View reviewed changes

fix omitempty on ProcMount

5e63ef0

Signed-off-by: Jess Frazelle <[email protected]>

jessfraz force-pushed the 69647 branch from feb0c60 to 5080b08 Compare October 12, 2018 16:14

k8s-ci-robot removed the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Oct 12, 2018

k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. labels Oct 12, 2018

run make update

ce6f716

Signed-off-by: Jess Frazelle <[email protected]>

jessfraz force-pushed the 69647 branch from 5080b08 to ce6f716 Compare October 12, 2018 16:16

k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Oct 12, 2018

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 12, 2018

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 12, 2018

jessfraz mentioned this pull request Oct 12, 2018

cherrypick for #69694 #69744

Merged

k8s-ci-robot merged commit 74d0ef2 into kubernetes:master Oct 12, 2018

jessfraz deleted the 69647 branch October 12, 2018 23:02

k8s-ci-robot added a commit that referenced this pull request Oct 13, 2018

Merge pull request #69744 from jessfraz/cherrypick-69694

5cda1f3

cherrypick for #69694

liggitt mentioned this pull request Oct 15, 2018

Add linting test for omitempty kubernetes/kube-openapi#106

Merged

AkihiroSuda mentioned this pull request Dec 27, 2018

(deleted) moby/buildkit#762

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add default for ProcMountType #69694

add default for ProcMountType #69694

jessfraz commented Oct 11, 2018 •

edited by liggitt

Loading

jessfraz commented Oct 11, 2018

dims commented Oct 11, 2018

jessfraz commented Oct 11, 2018

jessfraz commented Oct 11, 2018

liggitt commented Oct 11, 2018 •

edited

Loading

liggitt Oct 11, 2018

MarcPow Oct 12, 2018 •

edited

Loading

jessfraz Oct 12, 2018

jessfraz commented Oct 12, 2018

liggitt commented Oct 12, 2018

liggitt commented Oct 12, 2018

k8s-ci-robot commented Oct 12, 2018

jessfraz commented Oct 12, 2018 via email

jessfraz commented Oct 12, 2018

liggitt commented Oct 15, 2018

add default for ProcMountType #69694

add default for ProcMountType #69694

Conversation

jessfraz commented Oct 11, 2018 • edited by liggitt Loading

jessfraz commented Oct 11, 2018

dims commented Oct 11, 2018

jessfraz commented Oct 11, 2018

jessfraz commented Oct 11, 2018

liggitt commented Oct 11, 2018 • edited Loading

liggitt Oct 11, 2018

Choose a reason for hiding this comment

MarcPow Oct 12, 2018 • edited Loading

Choose a reason for hiding this comment

jessfraz Oct 12, 2018

Choose a reason for hiding this comment

jessfraz commented Oct 12, 2018

liggitt commented Oct 12, 2018

liggitt commented Oct 12, 2018

k8s-ci-robot commented Oct 12, 2018

jessfraz commented Oct 12, 2018 via email

jessfraz commented Oct 12, 2018

liggitt commented Oct 15, 2018

jessfraz commented Oct 11, 2018 •

edited by liggitt

Loading

liggitt commented Oct 11, 2018 •

edited

Loading

MarcPow Oct 12, 2018 •

edited

Loading