control-plane icon indicating copy to clipboard operation
control-plane copied to clipboard
verified publisher icon kyma-project

Fix Provisioner cluster role binding creation process

Open koala7659 opened this issue 2 years ago • 6 comments

This is a Follow Up story after fixing issue 1912:

Description:

Provisioner is responsible for initial setup of the created Kyma Runtime.

There was once introduced functionality to dynamically update list of runtime administrators by the Provisioner during Upgrade Shoot API call. This code was supposed to create and label required ClusterRoles and ClusterRoleBindings to generate correct set of administrators accounts during creation/update of the managed Kyma Runtime.

Code responsible for creating ClusterRoles and ClusterRole bindings can be found here: https://github.com/kyma-project/control-plane/blob/main/components/provisioner/internal/operations/stages/provisioning/create_operators_bindings.go

Problems with code:

  • Mixed responsibility - same step is evenly executed after creation of Kyma Runtime and when Gardener Shoot is updated. Expected results are different. This can be treated as of violation Single Responsibility Principle.
  • Code does not works to handle correctly updates of list of SKR administrators. Bindings must be added manually by the first administrator when new administrators. It is not obvious for the user. It is possible that provisioner API is not called properly when the list of subaccount administrator is changed
  • Created ClusterRules are not labeled according to convention with label reconciler.kyma-project.io/managed-by=reconciler
  • Code is not covered properly with Unit Tests

Suggested solution:

  1. Check with other teams what is the current state of functionality to update list of administrators of SKR with Provisioner.
  2. Refactor the code. A. If Provisioner is not able to dynamically update runtime bindings - consider removing execution path for shoot upgrade. B. If Provisioner is somehow able to dynamically update runtime bindings - Consider refactoring of the code by splitting it into two separate operations executed on different stages of Kyma runtime
  3. Add label reconciler.kyma-project.io/managed-by=reconciler for created ClusterRules
  4. Cover code with unit tests

[Moved from Provisioner to KIM]

koala7659 avatar Jun 14 '23 13:06 koala7659

This issue or PR has been automatically marked as stale due to the lack of recent activity. Thank you for your contributions.

This bot triages issues and PRs according to the following rules:

  • After 60d of inactivity, lifecycle/stale is applied
  • After 7d of inactivity since lifecycle/stale was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Close this issue or PR with /close

If you think that I work incorrectly, kindly raise an issue with the problem.

/lifecycle stale

kyma-bot avatar Sep 26 '23 10:09 kyma-bot

This issue or PR has been automatically closed due to the lack of activity. Thank you for your contributions.

This bot triages issues and PRs according to the following rules:

  • After 60d of inactivity, lifecycle/stale is applied
  • After 7d of inactivity since lifecycle/stale was applied, the issue is closed

You can:

  • Reopen this issue or PR with /reopen
  • Mark this issue or PR as fresh with /remove-lifecycle stale

If you think that I work incorrectly, kindly raise an issue with the problem.

/close

kyma-bot avatar Oct 03 '23 11:10 kyma-bot

@kyma-bot: Closing this issue.

In response to this:

This issue or PR has been automatically closed due to the lack of activity. Thank you for your contributions.

This bot triages issues and PRs according to the following rules:

  • After 60d of inactivity, lifecycle/stale is applied
  • After 7d of inactivity since lifecycle/stale was applied, the issue is closed

You can:

  • Reopen this issue or PR with /reopen
  • Mark this issue or PR as fresh with /remove-lifecycle stale

If you think that I work incorrectly, kindly raise an issue with the problem.

/close

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.

kyma-bot avatar Oct 03 '23 11:10 kyma-bot

This issue or PR has been automatically closed due to the lack of activity. Thank you for your contributions.

This bot triages issues and PRs according to the following rules:

  • After 60d of inactivity, lifecycle/stale is applied
  • After 7d of inactivity since lifecycle/stale was applied, the issue is closed

You can:

  • Reopen this issue or PR with /reopen
  • Mark this issue or PR as fresh with /remove-lifecycle stale

If you think that I work incorrectly, kindly raise an issue with the problem.

/close

kyma-bot avatar Oct 13 '23 09:10 kyma-bot

@kyma-bot: Closing this issue.

In response to this:

This issue or PR has been automatically closed due to the lack of activity. Thank you for your contributions.

This bot triages issues and PRs according to the following rules:

  • After 60d of inactivity, lifecycle/stale is applied
  • After 7d of inactivity since lifecycle/stale was applied, the issue is closed

You can:

  • Reopen this issue or PR with /reopen
  • Mark this issue or PR as fresh with /remove-lifecycle stale

If you think that I work incorrectly, kindly raise an issue with the problem.

/close

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.

kyma-bot avatar Oct 13 '23 09:10 kyma-bot

@koala7659 : could we keep this closed as the provision has reached End-Of-Life soon?

tobiscr avatar Dec 13 '23 12:12 tobiscr

@tobiscr I think we should close this. This code is being gradually removed (. https://github.com/kyma-project/control-plane/pull/3438/files) and soon will be fully replaced with new infrastructure-manager service

koala7659 avatar May 20 '24 13:05 koala7659