Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

⚠️ deprecate component config and no longer able to ensure that it is functioning as intended #3442

Merged

Conversation

camilamacedo86
Copy link
Member

Description

The ComponentConfig has been deprecated in the Controller-Runtime since its version 0.15.0. More info
Moreover, it has undergone breaking changes and is no longer functioning as intended.
As a result, Kubebuilder, which heavily relies on the Controller Runtime, has also deprecated this feature,
no longer guaranteeing its functionality from version 3.11.0 onwards. You can find additional details on this issue here.

Please, be aware that it will force Kubebuilder remove this option in soon future release.

Motivation

Bump Controller Runtime v0.15.0 and add support for k8s 1.27

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jun 9, 2023
@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jun 9, 2023
@camilamacedo86 camilamacedo86 reopened this Jun 9, 2023
@kubernetes-sigs kubernetes-sigs deleted a comment from k8s-ci-robot Jun 9, 2023
@@ -136,6 +136,5 @@ scaffold_test_project project-v3 --plugins="go/v3"
scaffold_test_project project-v4 --plugins="go/v4"
scaffold_test_project project-v4-multigroup --plugins="go/v4"
scaffold_test_project project-v4-declarative-v1 --plugins="go/v4,declarative"
scaffold_test_project project-v4-config --component-config --plugins="go/v4"
Copy link
Member

Choose a reason for hiding this comment

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

Should we change this to scaffold_test_project project-v4-config --plugins="go/v4" ? Just remove component-config

Copy link
Member Author

Choose a reason for hiding this comment

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

this line scaffolds the sample under testdata with component config
So we are just removing here since we no longer need this scaffold

Copy link
Member

Choose a reason for hiding this comment

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

That's ok.

Copy link
Member

@kkkkun kkkkun left a comment

Choose a reason for hiding this comment

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

LGTM

@kkkkun
Copy link
Member

kkkkun commented Jun 9, 2023

Hope to confirm again @Kavinjsir @varshaprasad96

Copy link
Member

@varshaprasad96 varshaprasad96 left a comment

Choose a reason for hiding this comment

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

The changes look good to me. But, we may also have to deprecate/remove the following:

  1. Setting of component config flag:
    func (c *cfg) SetComponentConfig() error {
  2. Remove the option for setting the flag in kustomize plugin:
    fs.BoolVar(&p.componentConfig, "component-config", false,
    and
    if p.componentConfig {
    if err := p.config.SetComponentConfig(); err != nil {
    return err
    }
    }

Comment on lines 6 to 11
The ComponentConfig has been deprecated in the Controller-Runtime since its version 0.15.0. [More info](https://github.com/kubernetes-sigs/controller-runtime/issues/895)
Moreover, it has undergone breaking changes and is no longer functioning as intended.
As a result, Kubebuilder, which heavily relies on the Controller Runtime, has also deprecated this feature,
no longer guaranteeing its functionality from version 3.11.0 onwards. You can find additional details on this issue [here](https://github.com/kubernetes-sigs/controller-runtime/issues/2370).

Please, be aware that it will force Kubebuilder remove this option in soon future release.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The ComponentConfig has been deprecated in the Controller-Runtime since its version 0.15.0. [More info](https://github.com/kubernetes-sigs/controller-runtime/issues/895)
Moreover, it has undergone breaking changes and is no longer functioning as intended.
As a result, Kubebuilder, which heavily relies on the Controller Runtime, has also deprecated this feature,
no longer guaranteeing its functionality from version 3.11.0 onwards. You can find additional details on this issue [here](https://github.com/kubernetes-sigs/controller-runtime/issues/2370).
Please, be aware that it will force Kubebuilder remove this option in soon future release.
The `ComponentConfig` feature has been deprecated in the controller-Runtime since its version of 0.15.0. [More info](https://github.com/kubernetes-sigs/controller-runtime/issues/895)
Moreover, it has undergone breaking changes and is no longer functioning as intended.
As a result, Kubebuilder, which heavily relies on Controller Runtime, has also deprecated this feature, and no longer guarantees to have its functionality from version 3.11.0 and above. You can find additional details on this [issue](https://github.com/kubernetes-sigs/controller-runtime/issues/2370).
Please be aware that it will force Kubebuilder to remove this option in future release if controller-runtime decides to not support it further.
Instead of using component-config, the [`Options`](https://github.com/kubernetes-sigs/controller-runtime/blob/e54088c8c7da82111b4508bdaf189c45d1344f00/pkg/manager/manager.go#L100) can be passed directly to the manager like below:
```go
mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), manager.Options{
Scheme: scheme,
Controller: config.Controller{
NeedLeaderElection: pointer.Bool(true),
MaxConcurrentReconciles: 2,
...
},
})

Copy link
Member

Choose a reason for hiding this comment

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

The alternative is straight forward, hence added it here. It would be helpful to provide another approach if we are deprecating this one.

Copy link
Member Author

Choose a reason for hiding this comment

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

@varshaprasad96 but the idea of the ComponentConfig is we are able to have an option to deploy the same solution (same code) with different config options per env. So, if we pass direct the Options in the manager (default behaviour) then it does not achieve the same goal. Right?

Therefore, should we really say that?

Copy link
Member

Choose a reason for hiding this comment

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

Since passing of the configuration options through the config file s removed, the next alternative for it to pass it through manager.Options. The idea is letting the user pass all customizable options through manager instead of config file. Which is why it is the alternative to what we have currently.

We need not block this PR for the docs update, I can add that in a follow up.

Comment on lines 6 to 11
The ComponentConfig has been deprecated in the Controller-Runtime since its version 0.15.0. [More info](https://github.com/kubernetes-sigs/controller-runtime/issues/895)
Moreover, it has undergone breaking changes and is no longer functioning as intended.
As a result, Kubebuilder, which heavily relies on the Controller Runtime, has also deprecated this feature,
no longer guaranteeing its functionality from version 3.11.0 onwards. You can find additional details on this issue [here](https://github.com/kubernetes-sigs/controller-runtime/issues/2370).

Please, be aware that it will force Kubebuilder remove this option in soon future release.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The ComponentConfig has been deprecated in the Controller-Runtime since its version 0.15.0. [More info](https://github.com/kubernetes-sigs/controller-runtime/issues/895)
Moreover, it has undergone breaking changes and is no longer functioning as intended.
As a result, Kubebuilder, which heavily relies on the Controller Runtime, has also deprecated this feature,
no longer guaranteeing its functionality from version 3.11.0 onwards. You can find additional details on this issue [here](https://github.com/kubernetes-sigs/controller-runtime/issues/2370).
Please, be aware that it will force Kubebuilder remove this option in soon future release.
The ComponentConfig has been deprecated in the Controller-Runtime since its version 0.15.0. [More info](https://github.com/kubernetes-sigs/controller-runtime/issues/895)
Moreover, it has undergone breaking changes and is no longer functioning as intended.
As a result, Kubebuilder, which heavily relies on the Controller Runtime, has also deprecated this feature,
no longer guaranteeing its functionality from version 3.11.0 onwards. You can find additional details on this issue [here](https://github.com/kubernetes-sigs/controller-runtime/issues/2370).
Please, be aware that it will force Kubebuilder remove this option soon in future releases.

Comment on lines 6 to 11
The ComponentConfig has been deprecated in the Controller-Runtime since its version 0.15.0. [More info](https://github.com/kubernetes-sigs/controller-runtime/issues/895)
Moreover, it has undergone breaking changes and is no longer functioning as intended.
As a result, Kubebuilder, which heavily relies on the Controller Runtime, has also deprecated this feature,
no longer guaranteeing its functionality from version 3.11.0 onwards. You can find additional details on this issue [here](https://github.com/kubernetes-sigs/controller-runtime/issues/2370).

Please, be aware that it will force Kubebuilder remove this option in soon future release.
Copy link
Member

Choose a reason for hiding this comment

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

Nit

Suggested change
The ComponentConfig has been deprecated in the Controller-Runtime since its version 0.15.0. [More info](https://github.com/kubernetes-sigs/controller-runtime/issues/895)
Moreover, it has undergone breaking changes and is no longer functioning as intended.
As a result, Kubebuilder, which heavily relies on the Controller Runtime, has also deprecated this feature,
no longer guaranteeing its functionality from version 3.11.0 onwards. You can find additional details on this issue [here](https://github.com/kubernetes-sigs/controller-runtime/issues/2370).
Please, be aware that it will force Kubebuilder remove this option in soon future release.
The ComponentConfig has been deprecated in the Controller-Runtime since its version 0.15.0. [More info](https://github.com/kubernetes-sigs/controller-runtime/issues/895)
Moreover, it has undergone breaking changes and is no longer functioning as intended.
As a result, Kubebuilder, which heavily relies on the Controller Runtime, has also deprecated this feature,
no longer guaranteeing its functionality from version 3.11.0 onwards. You can find additional details on this issue [here](https://github.com/kubernetes-sigs/controller-runtime/issues/2370).
Please, be aware that it will force Kubebuilder remove this option soon in future releases.

Comment on lines 6 to 11
The ComponentConfig has been deprecated in the Controller-Runtime since its version 0.15.0. [More info](https://github.com/kubernetes-sigs/controller-runtime/issues/895)
Moreover, it has undergone breaking changes and is no longer functioning as intended.
As a result, Kubebuilder, which heavily relies on the Controller Runtime, has also deprecated this feature,
no longer guaranteeing its functionality from version 3.11.0 onwards. You can find additional details on this issue [here](https://github.com/kubernetes-sigs/controller-runtime/issues/2370).

Please, be aware that it will force Kubebuilder remove this option in soon future release.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The ComponentConfig has been deprecated in the Controller-Runtime since its version 0.15.0. [More info](https://github.com/kubernetes-sigs/controller-runtime/issues/895)
Moreover, it has undergone breaking changes and is no longer functioning as intended.
As a result, Kubebuilder, which heavily relies on the Controller Runtime, has also deprecated this feature,
no longer guaranteeing its functionality from version 3.11.0 onwards. You can find additional details on this issue [here](https://github.com/kubernetes-sigs/controller-runtime/issues/2370).
Please, be aware that it will force Kubebuilder remove this option in soon future release.
The ComponentConfig has been deprecated in the Controller-Runtime since its version 0.15.0. [More info](https://github.com/kubernetes-sigs/controller-runtime/issues/895)
Moreover, it has undergone breaking changes and is no longer functioning as intended.
As a result, Kubebuilder, which heavily relies on the Controller Runtime, has also deprecated this feature,
no longer guaranteeing its functionality from version 3.11.0 onwards. You can find additional details on this issue [here](https://github.com/kubernetes-sigs/controller-runtime/issues/2370).
Please, be aware that it will force Kubebuilder remove this option soon in future releases.

Copy link
Member

@varshaprasad96 varshaprasad96 left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 12, 2023
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: camilamacedo86, kkkkun, varshaprasad96

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 [camilamacedo86,varshaprasad96]

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

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. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants