🌱 supervisor: drop ProviderServiceAccount ConfigMap and deprecate ProviderServiceAccountFinalizer #3009
Conversation
|
/test help |
k8s-ci-robot
added
the
cncf-cla: yes
|
@chrischdi: The specified target(s) for
The following commands are available to trigger optional jobs:
Use
In response to this:
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-sigs/prow repository. |
|
/test pull-cluster-api-provider-vsphere-e2e-supervisor-main |
k8s-ci-robot
added
the
size/L
|
/retest unrelated flake |
|
@chrischdi: The
The following commands are available to trigger optional jobs:
Use
In response to this:
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-sigs/prow repository. |
|
/retest |
chrischdi
changed the title
|
/override pull-cluster-api-provider-vsphere-apidiff-main |
|
@chrischdi: Overrode contexts on behalf of chrischdi: pull-cluster-api-provider-vsphere-apidiff-main In response to this:
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-sigs/prow repository. |
|
Apidiff shouldn't need an override |
| @@ -184,7 +182,8 @@ func (r *ServiceAccountReconciler) Reconcile(ctx context.Context, req reconcile. | |||
| }() | |||
|
|
|||
| if !vsphereCluster.DeletionTimestamp.IsZero() { | |||
| return ctrl.Result{}, r.reconcileDelete(ctx, clusterContext) | |||
| controllerutil.RemoveFinalizer(clusterContext.VSphereCluster, vmwarev1.ProviderServiceAccountFinalizer) | |||
There was a problem hiding this comment.
I assume it's okay that someone else is responsible to cleanup existing ConfigMaps? (I think it's fine, just want to double check)
There was a problem hiding this comment.
Looks like we never created it, so I think it should be okay that "whoever creates it" should clean it up once we don't use it anymore
There was a problem hiding this comment.
Yeah its precreated and you have to pass in env vars for its name and namespace.
There was a problem hiding this comment.
What we could do though is deleting it if the env vars are set and it still exists.
There was a problem hiding this comment.
I would probably prefer if we just tell people to delete it. The way some people upgrade (jumping over various versions) we have to keep this forever otherwise
I know, just looks cleaner in notifications :-) |
|
/lgtm /assign |
k8s-ci-robot
added
the
lgtm
|
LGTM label has been added. Git tree hash: 04627b6cff3ea215f24c2a9c8c046bcff8f822c3
|
|
/assign @fabriziopandini |
k8s-ci-robot
removed
the
lgtm
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
LGTM label has been added. Git tree hash: 087d1d5cd6202e4302b70d94db2761e4fa04eeba
|
|
/lgtm |
k8s-ci-robot
added
needs-rebase
…derServiceAccountFinalizer
chrischdi
force-pushed
the
pr-svs-drop-svcacct-cm
branch
from
8a83d2b
to
8a74868
Compare
k8s-ci-robot
removed
lgtm
|
rebased + squashed |
|
@chrischdi: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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-sigs/prow repository. I understand the commands that are listed here. |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
LGTM label has been added. Git tree hash: d627be95be876de21d977d8a5c51b8fb7770b46e
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fabriziopandini 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:
Approvers can indicate their approval by writing |
chrischdi
changed the title
What this PR does / why we need it:
Drops the functionality of recording created ProviderServiceAccounts in a ConfigMap because this is not used anymore on dependent controllers.
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #2861