Skip to content

Conversation

@tekenstam
Copy link
Member

@tekenstam tekenstam commented Apr 13, 2025

Overview

This PR implements comprehensive updates to the codebase including Kubernetes dependency upgrades, linting fixes, and test coverage improvements. These changes align with modern Go practices and the Keikoproj development guidelines while maintaining backward compatibility.

Resolves #466

Key Changes

Dependency and Tooling Updates

  • Updated Kubernetes dependencies to newer versions:
    • controller-runtime from v0.14.6 to v0.20.4
    • code-generator to v0.32.3
    • kube-openapi and related k8s.io packages
  • Upgraded Go version to 1.24
  • Updated controller-gen and golangci-lint tooling

Code Quality Improvements

  • Fixed potential nil pointer dereferences through proper nil checks
  • Added proper error handling for previously unchecked function calls
  • Removed deprecated imports (replaced io/ioutil with os package)
  • Eliminated unused variables and functions
  • Improved loop efficiency with idiomatic Go patterns
  • Standardized error handling patterns

CI/CD Enhancements

  • Added golangci-lint to the push workflow
  • Updated GitHub Actions and workflow configurations
  • Enforced stricter linting rules in CI

Testing Improvements

  • Added unit tests for controller reconcilers (spot events, nodes, configmaps, namespaces)
  • Created tests for utility functions in common and kubernetes packages
  • Improved test infrastructure for better reliability

Benefits

  • More up-to-date and secure dependencies
  • Better code quality and reliability through modern practices
  • Reduced potential for runtime errors
  • Better error handling and debugging capabilities
  • Enforced linting in CI prevents regression
  • Increased test coverage validates code behavior

Testing

  • All unit tests are passing
  • Linting now passes without warnings/errors
  • Tested with updated Go 1.24 and dependencies
  • Verified backward compatibility

This PR resolves #466 by addressing the identified linting issues while also modernizing the codebase through dependency updates and improved testing.

- Add proper error handling for AddToScheme calls in main.go
- Add error handling for ScalingConfiguration.Delete calls
- Replace deprecated strings.Title with cases.Title
- Fix tautological condition in GetUpgradeStrategy

Fixes #466

Signed-off-by: Todd Ekenstam <[email protected]>
@tekenstam tekenstam requested review from a team as code owners April 13, 2025 02:47
@codecov-commenter
Copy link

codecov-commenter commented Apr 13, 2025

Codecov Report

Attention: Patch coverage is 37.50000% with 45 lines in your changes missing coverage. Please review.

Project coverage is 46.11%. Comparing base (a3d330a) to head (fb2234f).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
controllers/reconcilers.go 0.00% 25 Missing ⚠️
controllers/providers/aws/ec2.go 0.00% 4 Missing ⚠️
controllers/providers/aws/predicates.go 0.00% 4 Missing ⚠️
controllers/provisioners/eks/cloud.go 25.00% 2 Missing and 1 partial ⚠️
controllers/provisioners/eks/helpers.go 72.72% 2 Missing and 1 partial ⚠️
controllers/providers/kubernetes/utils.go 85.71% 2 Missing ⚠️
api/instancemgr/v1alpha1/instancegroup_types.go 66.66% 1 Missing ⚠️
controllers/common/utils.go 66.66% 1 Missing ⚠️
controllers/providers/aws/iam.go 0.00% 1 Missing ⚠️
...rollers/provisioners/eks/scaling/launchtemplate.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #467      +/-   ##
==========================================
+ Coverage   44.99%   46.11%   +1.12%     
==========================================
  Files          40       40              
  Lines        7017     7007      -10     
==========================================
+ Hits         3157     3231      +74     
+ Misses       3711     3620      -91     
- Partials      149      156       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Signed-off-by: Todd Ekenstam <[email protected]>
@tekenstam tekenstam changed the title Fix golangci-lint issues Code modernization and lint fixes Apr 13, 2025
@tekenstam tekenstam changed the title Code modernization and lint fixes chore: update golang dependencies, fix linting issues, and improve test coverage Apr 13, 2025
Copy link

@shaoxt shaoxt left a comment

Choose a reason for hiding this comment

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

LGTM

@tekenstam tekenstam merged commit 49f7a7d into master Apr 13, 2025
3 checks passed
@tekenstam tekenstam deleted the fix-lint-issues branch April 13, 2025 20:10
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.

[BUG] Fix golangci-lint issues to improve code quality

5 participants