Skip to content

Conversation

@wking
Copy link
Member

@wking wking commented Sep 20, 2021

This content is injected by an admission webhook. When we started removing not-in-manifest volumes in 83faa6e (#654), the cluster-version operator started removing the webhook-injected volume, leading to the cluster-version operator crash-looping on updates from 4.8 to 4.9 with messages like:

F0920 13:23:23.565439       1 start.go:24] error: error creating clients: invalid configuration: no configuration has been provided, try setting KUBERNETES_MASTER environment variable

With this commit, we follow the precedent of the Kubernetes API server's own manifest: openshift/cluster-kube-apiserver-operator#1142.

@openshift-ci openshift-ci bot added bugzilla/severity-urgent Referenced Bugzilla bug's severity is urgent for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. labels Sep 20, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 20, 2021

@wking: This pull request references Bugzilla bug 2005581, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.10.0) matches configured target release for branch (4.10.0)
  • bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @jianlinliu

In response to this:

Bug 2005581: install/0000_00_cluster-version-operator_03_deployment: Explicit kube-api-access

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 20, 2021
@wking wking force-pushed the explicit-kube-api-access-volume branch from 5bbd270 to b411810 Compare September 21, 2021 03:24
…-api-access

This content is injected by an admission webhook [1,2].  When we
started removing not-in-manifest volumes in 83faa6e
(lib/resourcemerge/core: Remove unrecognized volumes and mounts,
2021-09-14, openshift#654), the cluster-version operator started removing the
webhook-injected volume, leading to the cluster-version operator
crash-looping on updates from 4.8 to 4.9 with messages like [3]:

  F0920 13:23:23.565439       1 start.go:24] error: error creating clients: invalid configuration: no configuration has been provided, try setting KUBERNETES_MASTER environment variable

With this commit, we follow the precedent of the Kubernetes API
server's own manifest [4,5].

[1]: https://github.com/kubernetes/kubernetes/blob/2f68346fbb6246961ce0a3176418630950aea500/plugin/pkg/admission/serviceaccount/admission.go#L53-L54
[2]: https://kubernetes.io/docs/reference/access-authn-authz/service-accounts-admin/#bound-service-account-token-volume
[3]: https://bugzilla.redhat.com/show_bug.cgi?id=2005581
[4]: openshift/cluster-kube-apiserver-operator#1142
[5]: https://bugzilla.redhat.com/show_bug.cgi?id=1946479
@wking wking force-pushed the explicit-kube-api-access-volume branch from b411810 to 97289cc Compare September 21, 2021 03:45
@LalatenduMohanty
Copy link
Member

@wking in the openshift/cluster-kube-apiserver-operator#1142 , I see a comment about automountServiceAccountToken: false # here to prevent deadlock, remove in 4.9 . But I am not sure why it is suggested to remove automountServiceAccountToken: false . However it is present in 4.9 [1]
[1] https://github.com/openshift/cluster-kube-apiserver-operator/blob/release-4.9/manifests/0000_20_kube-apiserver-operator_06_deployment.yaml#L27

Copy link
Member

@LalatenduMohanty LalatenduMohanty left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 21, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 21, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: LalatenduMohanty, wking

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:
  • OWNERS [LalatenduMohanty,wking]

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

@sdodson
Copy link
Member

sdodson commented Sep 21, 2021

/test e2e-agnostic-operator
/test e2e-agnostic-upgrade

@wking
Copy link
Member Author

wking commented Sep 21, 2021

Neither failure looked related. But I'll let the new round run out, and consider /override if they fail too...

@openshift-merge-robot openshift-merge-robot merged commit e816c11 into openshift:master Sep 21, 2021
@wking wking deleted the explicit-kube-api-access-volume branch September 21, 2021 17:37
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 21, 2021

@wking: All pull requests linked via external trackers have merged:

Bugzilla bug 2005581 has been moved to the MODIFIED state.

In response to this:

Bug 2005581: install/0000_00_cluster-version-operator_03_deployment: Explicit kube-api-access

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. bugzilla/severity-urgent Referenced Bugzilla bug's severity is urgent for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants