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

refactor: map false minikube unused resources #241

Merged
merged 1 commit into from
Apr 30, 2024

Conversation

cecobask
Copy link
Contributor

Fixes #238

Comment on lines +159 to +165
slicesToAppend := [][]string{
volumesCM,
envCM,
envFromCM,
envFromContainerCM,
envFromInitContainerCM,
}
Copy link
Contributor Author

@cecobask cecobask Apr 18, 2024

Choose a reason for hiding this comment

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

This is automatic formatting by my IDE (GoLand). There are multiple similar changes across the files.
Let me know if this is okay. If not, I can undo the formatting changes.
Ideally, the code style should be enforced by rules. Possibly, a target in a Makefile using gofmt, gofumpt?

Copy link
Owner

Choose a reason for hiding this comment

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

Yeah. We need to enforce it in the CI.
I'll open a task for it

Copy link
Owner

Choose a reason for hiding this comment

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

Strange. Intellij doesn't reformat to this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There might be some differences in how IntelliJ and GoLand work when it comes to formatting. I’m using the default code style configuration, no custom tweaks there.

@cecobask cecobask marked this pull request as ready for review April 18, 2024 16:20
pkg/kor/kor.go Show resolved Hide resolved
pkg/kor/storageclasses.go Show resolved Hide resolved
Comment on lines +159 to +165
slicesToAppend := [][]string{
volumesCM,
envCM,
envFromCM,
envFromContainerCM,
envFromInitContainerCM,
}
Copy link
Owner

Choose a reason for hiding this comment

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

Strange. Intellij doesn't reformat to this.

@doronkg
Copy link
Contributor

doronkg commented Apr 25, 2024

This PR also covers #235 almost completely.
I've created a default kind cluster and ran kor all right away.
All the unused ClusterRoles, ConfigMaps, Secrets & StorageClasses are mapped in this PR, but for one ClusterRole.

The ClusterRole system:persistent-volume-provisioner is created in both distros, but by default:

  • In minikube it is used by the storage-provisioner ClusterRoleBinding.
  • In kind it is unused by any RB/CRB.

@yonahd would you prefer that I'll submit a different PR for that or rather should we include the single difference in this PR?

@yonahd
Copy link
Owner

yonahd commented Apr 26, 2024

This PR also covers #235 almost completely. I've created a default kind cluster and ran kor all right away. All the unused ClusterRoles, ConfigMaps, Secrets & StorageClasses are mapped in this PR, but for one ClusterRole.

The ClusterRole system:persistent-volume-provisioner is created in both distros, but by default:

  • In minikube it is used by the storage-provisioner ClusterRoleBinding.
  • In kind it is unused by any RB/CRB.

@yonahd would you prefer that I'll submit a different PR for that or rather should we include the single difference in this PR?

@doronkg there are not some docker related configmaps specific to kind?

@doronkg
Copy link
Contributor

doronkg commented Apr 26, 2024

@doronkg there are not some docker related configmaps specific to kind?

All the ConfigMaps are the same between kind & minikube on default installations.

@yonahd
Copy link
Owner

yonahd commented Apr 26, 2024

@doronkg there are not some docker related configmaps specific to kind?

All the ConfigMaps are the same between kind & minikube on default installations.

Moving this discussion to the relevant ticket.

@cecobask
Copy link
Contributor Author

@yonahd, I rebased my branch with the latest changes and fixed the lint issues.
Please, take a look when you get a chance and let me know if something else is left to be done here 👍

@cecobask cecobask requested a review from yonahd April 30, 2024 09:12
Copy link
Owner

@yonahd yonahd left a comment

Choose a reason for hiding this comment

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

Lgtm
Thanks!

@yonahd yonahd merged commit 8822778 into yonahd:main Apr 30, 2024
1 check passed
@cecobask cecobask deleted the minikube-exceptions branch April 30, 2024 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Map false unused resources: minikube
3 participants