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

Do some code cleans. #1129

Merged
merged 1 commit into from
May 2, 2023
Merged

Do some code cleans. #1129

merged 1 commit into from
May 2, 2023

Conversation

yanggangtony
Copy link
Member

/kind cleanup

Its just some tiny code cleans..

@k8s-ci-robot k8s-ci-robot added the kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. label Apr 26, 2023
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 26, 2023
@k8s-ci-robot k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Apr 26, 2023
@@ -120,10 +120,10 @@ func setDefaultEvictor(profile api.DeschedulerProfile, client clientset.Interfac
if defaultevictorPluginConfig == nil {
profile.PluginConfigs = append([]api.PluginConfig{newPluginConfig}, profile.PluginConfigs...)
defaultevictorPluginConfig = &newPluginConfig
idx = 0
// when defaultevictorPluginConfig is nil , the idx=0
Copy link
Contributor

Choose a reason for hiding this comment

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

This assumes GetPluginConfig always returns idx == 0 for when defaultevictorPluginConfig == nil. That is another assumption that can be broken without noticing. GetPluginConfig could be later re-implement for non-sequential iteration and return anything other than 0 if not entry is found. Better keep it as it is.

Copy link
Member Author

Choose a reason for hiding this comment

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

thanks for review.
Will rollback it.

return errorsInProfiles
}
return nil
return errorsInProfiles
}

func setErrorsInProfiles(err error, profileName string, errorsInProfiles error) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

setErrorsInProfiles is used only once. Worth making it inline.

Copy link
Member Author

Choose a reason for hiding this comment

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

sound good.
update it.

@@ -140,33 +140,16 @@ func validateDeschedulerConfiguration(in api.DeschedulerPolicy, registry pluginr
pluginUtilities := registry[pluginConfig.Name]
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to fail in case registry[pluginConfig.Name] does not exists. E.g.:

if _, ok := registry[pluginConfig.Name]; !ok {
	errorsInProfiles = fmt.Errorf("%w: %s", errorsInProfiles, fmt.Sprintf("in profile %s: plugin %s in pluginConfig not registered", profile.Name, pluginConfig.Name))
	continue
}
pluginUtilities := registry[pluginConfig.Name]
if pluginUtilities.PluginArgValidator == nil {
	continue
}
err := pluginUtilities.PluginArgValidator(pluginConfig.Args)
...

This will also reduce the level of nesting by 2.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good suggestion.
Update later.

Signed-off-by: yanggang <gang.yang@daocloud.io>
@yanggangtony
Copy link
Member Author

@ingvagabund
Thanks for your review.
Rebased again.

@ingvagabund
Copy link
Contributor

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ingvagabund

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ingvagabund
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 2, 2023
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 2, 2023
@k8s-ci-robot k8s-ci-robot merged commit 5ba1fef into kubernetes-sigs:master May 2, 2023
@yanggangtony
Copy link
Member Author

thanks.
@ingvagabund

coolguy1771 pushed a commit to coolguy1771/home-ops that referenced this pull request May 6, 2023
This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [descheduler](https://togithub.com/kubernetes-sigs/descheduler) |
minor | `0.26.1` -> `0.27.0` |

---

### Release Notes

<details>
<summary>kubernetes-sigs/descheduler</summary>

###
[`v0.27.0`](https://togithub.com/kubernetes-sigs/descheduler/releases/tag/v0.27.0):
Descheduler v0.27.0

[Compare
Source](https://togithub.com/kubernetes-sigs/descheduler/compare/v0.26.1...v0.27.0)

#### What's Changed

- add v1alpha2 registry based conversion by
[@&#8203;knelasevero](https://togithub.com/knelasevero) in
[kubernetes-sigs/descheduler#1006
- support client connection configuration to provide QPS and burst by
[@&#8203;a7i](https://togithub.com/a7i) in
[kubernetes-sigs/descheduler#1034
- enable auto generation of the documents by
[@&#8203;harshanarayana](https://togithub.com/harshanarayana) in
[kubernetes-sigs/descheduler#1048
- Fix v1alpha1 conversion to use universal decoder by
[@&#8203;gustavomfc](https://togithub.com/gustavomfc) in
[kubernetes-sigs/descheduler#1051
- metrics: enable loop duration and strategy duration metrics by
[@&#8203;harshanarayana](https://togithub.com/harshanarayana) in
[kubernetes-sigs/descheduler#1041
- v1alpha2 docs by
[@&#8203;knelasevero](https://togithub.com/knelasevero) in
[kubernetes-sigs/descheduler#1049
- remove some logic useless by
[@&#8203;lucming](https://togithub.com/lucming) in
[kubernetes-sigs/descheduler#1059
- check pod number by
[@&#8203;xiaoanyunfei](https://togithub.com/xiaoanyunfei) in
[kubernetes-sigs/descheduler#1057
- Adding descheduler policy API Version option in helm templates by
[@&#8203;gdasson](https://togithub.com/gdasson) in
[kubernetes-sigs/descheduler#1068
- Fix typo in nodeutilization types by
[@&#8203;JannikSt](https://togithub.com/JannikSt) in
[kubernetes-sigs/descheduler#1072
- Add note to HighNodeUtilization readme for GKE users by
[@&#8203;damemi](https://togithub.com/damemi) in
[kubernetes-sigs/descheduler#1075
- expose security context from helm chart by
[@&#8203;JaneLiuL](https://togithub.com/JaneLiuL) in
[kubernetes-sigs/descheduler#1066
- remove e2e tests from helm github action by
[@&#8203;a7i](https://togithub.com/a7i) in
[kubernetes-sigs/descheduler#937
- Add k8s compatibility warnings to logs by
[@&#8203;JaneLiuL](https://togithub.com/JaneLiuL) in
[kubernetes-sigs/descheduler#1063
- docs: typo by [@&#8203;dangen-effy](https://togithub.com/dangen-effy)
in
[kubernetes-sigs/descheduler#1090
- Descheduling profile with PoC fake plugin by
[@&#8203;ingvagabund](https://togithub.com/ingvagabund) in
[kubernetes-sigs/descheduler#1093
- bump golangci for go 1.20 by [@&#8203;a7i](https://togithub.com/a7i)
in
[kubernetes-sigs/descheduler#1101
- dry-run cli flag to use uppercase by
[@&#8203;yanggangtony](https://togithub.com/yanggangtony) in
[kubernetes-sigs/descheduler#1103
- fix default value assignment of EvictLocalStoragePods by
[@&#8203;yanggangtony](https://togithub.com/yanggangtony) in
[kubernetes-sigs/descheduler#1104
- bump to k8s 1.27 rc0 by [@&#8203;a7i](https://togithub.com/a7i) in
[kubernetes-sigs/descheduler#1100
- fix go doc for method IsPodWithPVC by
[@&#8203;yanggangtony](https://togithub.com/yanggangtony) in
[kubernetes-sigs/descheduler#1105
- Populate extension points automatically for deschedule, balance,
filter and preEvictionFilter by
[@&#8203;ingvagabund](https://togithub.com/ingvagabund) in
[kubernetes-sigs/descheduler#1097
- Delete "io/ioutil" package. by
[@&#8203;yanggangtony](https://togithub.com/yanggangtony) in
[kubernetes-sigs/descheduler#1111
- Fix typo log message for cachedClient by
[@&#8203;yanggangtony](https://togithub.com/yanggangtony) in
[kubernetes-sigs/descheduler#1112
- bump to k8s 1.27 by [@&#8203;a7i](https://togithub.com/a7i) in
[kubernetes-sigs/descheduler#1115
- update v0.26.1 references by [@&#8203;a7i](https://togithub.com/a7i)
in
[kubernetes-sigs/descheduler#1109
- Drop Evict extension point by
[@&#8203;ingvagabund](https://togithub.com/ingvagabund) in
[kubernetes-sigs/descheduler#1120
- Add json tags to internal api fields by
[@&#8203;damemi](https://togithub.com/damemi) in
[kubernetes-sigs/descheduler#1122
- Add knelasevero to approvers by
[@&#8203;knelasevero](https://togithub.com/knelasevero) in
[kubernetes-sigs/descheduler#1130
- Drop remaining mentions of evict extension point by
[@&#8203;ingvagabund](https://togithub.com/ingvagabund) in
[kubernetes-sigs/descheduler#1127
- Add note on documentation versions with links by
[@&#8203;damemi](https://togithub.com/damemi) in
[kubernetes-sigs/descheduler#1132
- Add missing quote to configmap by
[@&#8203;damemi](https://togithub.com/damemi) in
[kubernetes-sigs/descheduler#1133
- fix policy example configmap by
[@&#8203;AllenZMC](https://togithub.com/AllenZMC) in
[kubernetes-sigs/descheduler#1135
- Do some code cleans. by
[@&#8203;yanggangtony](https://togithub.com/yanggangtony) in
[kubernetes-sigs/descheduler#1129

#### New Contributors

- [@&#8203;Bobonium](https://togithub.com/Bobonium) made their first
contribution in
[kubernetes-sigs/descheduler#995
- [@&#8203;harshanarayana](https://togithub.com/harshanarayana) made
their first contribution in
[kubernetes-sigs/descheduler#1048
- [@&#8203;gustavomfc](https://togithub.com/gustavomfc) made their first
contribution in
[kubernetes-sigs/descheduler#1051
- [@&#8203;lucming](https://togithub.com/lucming) made their first
contribution in
[kubernetes-sigs/descheduler#1059
- [@&#8203;gdasson](https://togithub.com/gdasson) made their first
contribution in
[kubernetes-sigs/descheduler#1068
- [@&#8203;JannikSt](https://togithub.com/JannikSt) made their first
contribution in
[kubernetes-sigs/descheduler#1072
- [@&#8203;dangen-effy](https://togithub.com/dangen-effy) made their
first contribution in
[kubernetes-sigs/descheduler#1090
- [@&#8203;yanggangtony](https://togithub.com/yanggangtony) made their
first contribution in
[kubernetes-sigs/descheduler#1103
- [@&#8203;AllenZMC](https://togithub.com/AllenZMC) made their first
contribution in
[kubernetes-sigs/descheduler#1135

**Full Changelog**:
kubernetes-sigs/descheduler@v0.26.0...v0.27.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Renovate
Bot](https://togithub.com/renovatebot/renovate).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS43MS41IiwidXBkYXRlZEluVmVyIjoiMzUuNzEuNSIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->

Co-authored-by: lumiere-bot <98047013+lumiere-bot[bot]@users.noreply.github.com>
renovate bot added a commit to teutonet/teutonet-helm-charts that referenced this pull request May 8, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [descheduler](https://togithub.com/kubernetes-sigs/descheduler) |
minor | `0.26.x` -> `0.27.x` |

---

### Release Notes

<details>
<summary>kubernetes-sigs/descheduler</summary>

###
[`v0.27.0`](https://togithub.com/kubernetes-sigs/descheduler/releases/tag/v0.27.0):
Descheduler v0.27.0

[Compare
Source](https://togithub.com/kubernetes-sigs/descheduler/compare/v0.26.1...v0.27.0)

#### What's Changed

- add v1alpha2 registry based conversion by
[@&#8203;knelasevero](https://togithub.com/knelasevero) in
[kubernetes-sigs/descheduler#1006
- support client connection configuration to provide QPS and burst by
[@&#8203;a7i](https://togithub.com/a7i) in
[kubernetes-sigs/descheduler#1034
- enable auto generation of the documents by
[@&#8203;harshanarayana](https://togithub.com/harshanarayana) in
[kubernetes-sigs/descheduler#1048
- Fix v1alpha1 conversion to use universal decoder by
[@&#8203;gustavomfc](https://togithub.com/gustavomfc) in
[kubernetes-sigs/descheduler#1051
- metrics: enable loop duration and strategy duration metrics by
[@&#8203;harshanarayana](https://togithub.com/harshanarayana) in
[kubernetes-sigs/descheduler#1041
- v1alpha2 docs by
[@&#8203;knelasevero](https://togithub.com/knelasevero) in
[kubernetes-sigs/descheduler#1049
- remove some logic useless by
[@&#8203;lucming](https://togithub.com/lucming) in
[kubernetes-sigs/descheduler#1059
- check pod number by
[@&#8203;xiaoanyunfei](https://togithub.com/xiaoanyunfei) in
[kubernetes-sigs/descheduler#1057
- Adding descheduler policy API Version option in helm templates by
[@&#8203;gdasson](https://togithub.com/gdasson) in
[kubernetes-sigs/descheduler#1068
- Fix typo in nodeutilization types by
[@&#8203;JannikSt](https://togithub.com/JannikSt) in
[kubernetes-sigs/descheduler#1072
- Add note to HighNodeUtilization readme for GKE users by
[@&#8203;damemi](https://togithub.com/damemi) in
[kubernetes-sigs/descheduler#1075
- expose security context from helm chart by
[@&#8203;JaneLiuL](https://togithub.com/JaneLiuL) in
[kubernetes-sigs/descheduler#1066
- remove e2e tests from helm github action by
[@&#8203;a7i](https://togithub.com/a7i) in
[kubernetes-sigs/descheduler#937
- Add k8s compatibility warnings to logs by
[@&#8203;JaneLiuL](https://togithub.com/JaneLiuL) in
[kubernetes-sigs/descheduler#1063
- docs: typo by [@&#8203;dangen-effy](https://togithub.com/dangen-effy)
in
[kubernetes-sigs/descheduler#1090
- Descheduling profile with PoC fake plugin by
[@&#8203;ingvagabund](https://togithub.com/ingvagabund) in
[kubernetes-sigs/descheduler#1093
- bump golangci for go 1.20 by [@&#8203;a7i](https://togithub.com/a7i)
in
[kubernetes-sigs/descheduler#1101
- dry-run cli flag to use uppercase by
[@&#8203;yanggangtony](https://togithub.com/yanggangtony) in
[kubernetes-sigs/descheduler#1103
- fix default value assignment of EvictLocalStoragePods by
[@&#8203;yanggangtony](https://togithub.com/yanggangtony) in
[kubernetes-sigs/descheduler#1104
- bump to k8s 1.27 rc0 by [@&#8203;a7i](https://togithub.com/a7i) in
[kubernetes-sigs/descheduler#1100
- fix go doc for method IsPodWithPVC by
[@&#8203;yanggangtony](https://togithub.com/yanggangtony) in
[kubernetes-sigs/descheduler#1105
- Populate extension points automatically for deschedule, balance,
filter and preEvictionFilter by
[@&#8203;ingvagabund](https://togithub.com/ingvagabund) in
[kubernetes-sigs/descheduler#1097
- Delete "io/ioutil" package. by
[@&#8203;yanggangtony](https://togithub.com/yanggangtony) in
[kubernetes-sigs/descheduler#1111
- Fix typo log message for cachedClient by
[@&#8203;yanggangtony](https://togithub.com/yanggangtony) in
[kubernetes-sigs/descheduler#1112
- bump to k8s 1.27 by [@&#8203;a7i](https://togithub.com/a7i) in
[kubernetes-sigs/descheduler#1115
- update v0.26.1 references by [@&#8203;a7i](https://togithub.com/a7i)
in
[kubernetes-sigs/descheduler#1109
- Drop Evict extension point by
[@&#8203;ingvagabund](https://togithub.com/ingvagabund) in
[kubernetes-sigs/descheduler#1120
- Add json tags to internal api fields by
[@&#8203;damemi](https://togithub.com/damemi) in
[kubernetes-sigs/descheduler#1122
- Add knelasevero to approvers by
[@&#8203;knelasevero](https://togithub.com/knelasevero) in
[kubernetes-sigs/descheduler#1130
- Drop remaining mentions of evict extension point by
[@&#8203;ingvagabund](https://togithub.com/ingvagabund) in
[kubernetes-sigs/descheduler#1127
- Add note on documentation versions with links by
[@&#8203;damemi](https://togithub.com/damemi) in
[kubernetes-sigs/descheduler#1132
- Add missing quote to configmap by
[@&#8203;damemi](https://togithub.com/damemi) in
[kubernetes-sigs/descheduler#1133
- fix policy example configmap by
[@&#8203;AllenZMC](https://togithub.com/AllenZMC) in
[kubernetes-sigs/descheduler#1135
- Do some code cleans. by
[@&#8203;yanggangtony](https://togithub.com/yanggangtony) in
[kubernetes-sigs/descheduler#1129

#### New Contributors

- [@&#8203;Bobonium](https://togithub.com/Bobonium) made their first
contribution in
[kubernetes-sigs/descheduler#995
- [@&#8203;harshanarayana](https://togithub.com/harshanarayana) made
their first contribution in
[kubernetes-sigs/descheduler#1048
- [@&#8203;gustavomfc](https://togithub.com/gustavomfc) made their first
contribution in
[kubernetes-sigs/descheduler#1051
- [@&#8203;lucming](https://togithub.com/lucming) made their first
contribution in
[kubernetes-sigs/descheduler#1059
- [@&#8203;gdasson](https://togithub.com/gdasson) made their first
contribution in
[kubernetes-sigs/descheduler#1068
- [@&#8203;JannikSt](https://togithub.com/JannikSt) made their first
contribution in
[kubernetes-sigs/descheduler#1072
- [@&#8203;dangen-effy](https://togithub.com/dangen-effy) made their
first contribution in
[kubernetes-sigs/descheduler#1090
- [@&#8203;yanggangtony](https://togithub.com/yanggangtony) made their
first contribution in
[kubernetes-sigs/descheduler#1103
- [@&#8203;AllenZMC](https://togithub.com/AllenZMC) made their first
contribution in
[kubernetes-sigs/descheduler#1135

**Full Changelog**:
kubernetes-sigs/descheduler@v0.26.0...v0.27.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://app.renovatebot.com/dashboard#github/teutonet/teutonet-helm-charts).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS43MS40IiwidXBkYXRlZEluVmVyIjoiMzUuNzEuNCIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
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. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants