{"action":"created","issue":{"url":"https://api.github.com/repos/kubernetes/helm/issues/4277","repository_url":"https://api.github.com/repos/kubernetes/helm","labels_url":"https://api.github.com/repos/kubernetes/helm/issues/4277/labels{/name}","comments_url":"https://api.github.com/repos/kubernetes/helm/issues/4277/comments","events_url":"https://api.github.com/repos/kubernetes/helm/issues/4277/events","html_url":"https://github.com/kubernetes/helm/issues/4277","id":336432927,"node_id":"MDU6SXNzdWUzMzY0MzI5Mjc=","number":4277,"title":"Hook annotations ignored on `PriorityClass` resource","user":{"login":"consideRatio","id":3837114,"node_id":"MDQ6VXNlcjM4MzcxMTQ=","avatar_url":"https://avatars3.githubusercontent.com/u/3837114?v=4","gravatar_id":"","url":"https://api.github.com/users/consideRatio","html_url":"https://github.com/consideRatio","followers_url":"https://api.github.com/users/consideRatio/followers","following_url":"https://api.github.com/users/consideRatio/following{/other_user}","gists_url":"https://api.github.com/users/consideRatio/gists{/gist_id}","starred_url":"https://api.github.com/users/consideRatio/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/consideRatio/subscriptions","organizations_url":"https://api.github.com/users/consideRatio/orgs","repos_url":"https://api.github.com/users/consideRatio/repos","events_url":"https://api.github.com/users/consideRatio/events{/privacy}","received_events_url":"https://api.github.com/users/consideRatio/received_events","type":"User","site_admin":false},"labels":[{"id":269212356,"node_id":"MDU6TGFiZWwyNjkyMTIzNTY=","url":"https://api.github.com/repos/kubernetes/helm/labels/question/support","name":"question/support","color":"cc317c","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":3,"created_at":"2018-06-28T00:29:31Z","updated_at":"2018-06-30T15:15:37Z","closed_at":null,"author_association":"CONTRIBUTOR","body":"## TL;DR\r\n1. Should Helm expedite loading of PriorityClass resources?\r\n2. Why doesn't Helm care about hook annotations on a PriorityClass resource?\r\n\r\n---\r\n\r\n## Issue 1 - Prioritized loading of PriorityClasses?\r\n\r\nBy adding a global [`PriorityClass`](https://kubernetes.io/docs/concepts/configuration/pod-priority-preemption/#priorityclass), all new pods without a `priorityClassName` will get some declared (pod-) `priority`.\r\n\r\n```yaml\r\napiVersion: scheduling.k8s.io/v1alpha1\r\nkind: PriorityClass\r\nmetadata:\r\n  name: global-priority\r\nvalue: 10\r\nglobalDefault: true\r\ndescription: \"A global priority for all new pods created after this is loaded.\"\r\n```\r\n\r\nBy including this `PriorityClass` in my chart, I hoped that all new object created would be assigned the global priority, but they wasn't until they were restarted manually after the `helm install` command. So I figure the `PriorityClass` was not loaded with expedition by Helm.\r\n\r\n#### Expedite loading of PriorityClasses\r\nDoes it make sense to expedite loading of PriorityClasses within Helm?\r\n\r\n---\r\n\r\n## Issue 2 - Helm ignores the hook annotation on PriorityClasses\r\nI attempted to work around this by forcing Helm to load it first using the prioritization possibilities using weights in Helm hooks. The lower weight value, the earlier you will be installed. It turns out this did not work out of the box.\r\n\r\n```yaml\r\napiVersion: scheduling.k8s.io/v1alpha1\r\nkind: PriorityClass\r\nmetadata:\r\n  name: global-priority\r\n  annotation:\r\n    # This global PriorityClass must be added first!\r\n    helm.sh/hook: pre-install,pre-upgrade\r\n    helm.sh/hook-delete-policy: before-hook-creation\r\n    helm.sh/hook-weight: \"-100\"\r\nvalue: 10\r\nglobalDefault: true\r\ndescription: \"A global priority for all new pods created after this is loaded.\"\r\n```\r\n\r\nI figured it might be because I also may require some time to pass in between this creation of the PriorityClass and the other pods. To accomplish this I created a Job with a pause container to run until I manually killed it after the PriorityClass. This still did not work! The PriorityClass will only be added after the blocking job is completed even though the PriorityClass is marked by the helm hook annotation to be loaded into k8s first!\r\n\r\nSo now I don't know whats going on any more... I've got this working with a different type of resource like a DaemonSet being loaded before a blocking Job.\r\n\r\n#### My research\r\n\r\nThis issue has not arisen when I do the exact same thing with a DaemonSet in another context. Is Helm ignores hook annotations on PriorityClasses simply handling the resource as if it was not a hook?\r\n\r\nYes - Helm is ignoring the hook annotation on this PriorityClass! I tested this by removing the `helm.sh/hook-delete-policy: before-hook-creation` annotation and making one fresh chart install followed by an update to the resource and helm treated it as an versioned update rather than a hook resource to be created! If it did, it would had complained about the resource already existing but it didn't! Instead of saying a resource was already having that name, I got the complaint that the `priority` field could not be updated.\r\n\r\nTo conclude I raise the question - *why does Helm not honor hook annotations on PriorityClass resources?*\r\n\r\n#### Note to self\r\nIt could be nice to use a helm hook with a `helm.sh/hook-delete-policy: before-hook-creation` annotation on PriorityClass resources to avoid using the `--force` flag on `helm upgrade` while making changes to it.\r\n\r\n## Environment\r\n\r\n```\r\n$ helm version --debug\r\nClient: &version.Version{SemVer:\"v2.9.1\", GitCommit:\"20adb27c7c5868466912eebdf6664e7390ebe710\", GitTreeState:\"clean\"}\r\nKubernetes: &version.Info{Major:\"1\", Minor:\"10+\", GitVersion:\"v1.10.4-gke.2\", GitCommit:\"eb2e43842aaa21d6f0bb65d6adf5a84bbdc62eaf\", GitTreeState:\"clean\", BuildDate:\"2018-06-15T21:48:39Z\", GoVersion:\"go1.9.3b4\", Compiler:\"gc\", Platform:\"linux/amd64\"}\r\nServer: &version.Version{SemVer:\"v2.9.1\", GitCommit:\"20adb27c7c5868466912eebdf6664e7390ebe710\", GitTreeState:\"clean\"}\r\n```\r\n\r\n## Reproduction output\r\n```\r\n$ helm install <the stuff below...>\r\n\r\n$ kubectl get priorityclass\r\nNo resources found.\r\n\r\n$ kubectl delete job post-default-priority-pause\r\njob.batch \"post-default-priority-pause\" deleted\r\n\r\n$ kubectl get priorityclass\r\nNAME              AGE\r\nglobal-priority   5s\r\n```\r\n\r\n## Reproduction resources\r\n\r\n```yaml\r\napiVersion: scheduling.k8s.io/v1alpha1\r\nkind: PriorityClass\r\nmetadata:\r\n  name: global-priority\r\n  annotation:\r\n    # This global PriorityClass must be added first.\r\n    helm.sh/hook: pre-install,pre-upgrade\r\n    helm.sh/hook-delete-policy: before-hook-creation\r\n    helm.sh/hook-weight: \"-100\"\r\nvalue: 10\r\nglobalDefault: true\r\ndescription: \"A default priority higher than the placeholder priority.\"\r\n---\r\napiVersion: batch/v1\r\nkind: Job\r\nmetadata:\r\n  name: post-default-priority-pause\r\n  annotations:\r\n    helm.sh/hook: pre-install\r\n    helm.sh/hook-delete-policy: before-hook-creation,hook-succeeded,hook-failed\r\n    helm.sh/hook-weight: \"-90\"\r\nspec:\r\n  template:\r\n    metadata:\r\n      labels:\r\n        key1: value1\r\n    spec:\r\n      restartPolicy: Never\r\n      containers:\r\n        - name: pause\r\n          image: gcr.io/google_containers/pause:3.1\r\n```"},"comment":{"url":"https://api.github.com/repos/kubernetes/helm/issues/comments/401547196","html_url":"https://github.com/kubernetes/helm/issues/4277#issuecomment-401547196","issue_url":"https://api.github.com/repos/kubernetes/helm/issues/4277","id":401547196,"node_id":"MDEyOklzc3VlQ29tbWVudDQwMTU0NzE5Ng==","user":{"login":"consideRatio","id":3837114,"node_id":"MDQ6VXNlcjM4MzcxMTQ=","avatar_url":"https://avatars3.githubusercontent.com/u/3837114?v=4","gravatar_id":"","url":"https://api.github.com/users/consideRatio","html_url":"https://github.com/consideRatio","followers_url":"https://api.github.com/users/consideRatio/followers","following_url":"https://api.github.com/users/consideRatio/following{/other_user}","gists_url":"https://api.github.com/users/consideRatio/gists{/gist_id}","starred_url":"https://api.github.com/users/consideRatio/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/consideRatio/subscriptions","organizations_url":"https://api.github.com/users/consideRatio/orgs","repos_url":"https://api.github.com/users/consideRatio/repos","events_url":"https://api.github.com/users/consideRatio/events{/privacy}","received_events_url":"https://api.github.com/users/consideRatio/received_events","type":"User","site_admin":false},"created_at":"2018-06-30T15:15:37Z","updated_at":"2018-06-30T15:15:37Z","author_association":"CONTRIBUTOR","body":"### I forgot the `s` on `annotations` and had silent failures...\r\n![picard-facepalm](https://user-images.githubusercontent.com/3837114/42126429-f575f69a-7c88-11e8-972a-284a51ba0ac2.jpg)\r\n\r\n---\r\n\r\n```yaml\r\nmetadata:\r\n  annotation:\r\n    key1: value1\r\n```"}}
{"actor":{"display_login":"consideRatio"}}