feat: cluster upgrade recommendation by satyampsoni · Pull Request #50 · devtron-labs/silver-surfer

satyampsoni · 2025-04-11T10:52:12Z

Add Migration File Generation Feature

Description

This PR adds a new feature to generate migration files when Kubernetes API versions change between different Kubernetes versions. The feature helps users automatically generate updated manifests with the correct API versions while preserving all other configurations.

Changes

Added new command-line flags:
- --generate-migration-files: Enable migration file generation
- --migration-output-dir: Specify output directory for migration files (default: "migrations")
Added Resource field to ValidationResult struct to store original resource data
Implemented MigrationFileGenerator to handle migration file creation
Modified validation process to preserve original resource data
Added migration file generation logic to both file and cluster processing

How it Works

When the --generate-migration-files flag is used, the tool:
- Validates resources against both source and target Kubernetes versions
- Identifies API version changes (e.g., extensions/v1beta1 → apps/v1)
- Generates migration files with updated API versions
- Preserves all other resource configurations
Migration files are generated with names following the format:
```
{namespace}-{name}-{kind}.yaml
```
Each migration file contains:
- Updated API version
- Original resource configuration
- Properly formatted metadata

Example Usage

./kubedd test-yamls/old-deployment.yaml \
  --generate-migration-files \
  --migration-output-dir ./migration-files \
  --source-kubernetes-version 1.30 \
  --target-kubernetes-version 1.31 \
  --source-schema-location ./schemas/1.30.json \
  --target-schema-location ./schemas/1.31.json

Testing

Tested with various resource types (Deployments, Ingress, Roles)
Confirmed preservation of resource configurations
Validated file naming and directory creation

Benefits

Automates the process of updating API versions
Reduces manual work in Kubernetes version upgrades
Preserves all resource configurations
Provides clear migration path for users

Related Issues

#13

prakash100198 · 2025-04-17T12:07:24Z

+	if generateMigrationFiles {
+		generator := pkg.NewMigrationFileGenerator(migrationOutputDir, noColor)
+		if err := generator.GenerateMigrationFiles(aggResults); err != nil {
+			log.Fatalf("Failed to generate migration files: %v", err)


don't use log.fatal here ,let's just print the error here using log2.Error(err) and return success as false , and let the callee handle the os.Exit part.

prakash100198 · 2025-04-17T12:20:00Z

+	RootCmd.Flags().BoolVar(&generateMigrationFiles, "generate-migration-files", false, "Generate migration files for API version updates")
+	RootCmd.Flags().StringVar(&migrationOutputDir, "migration-output-dir", "migrations", "Directory to store generated migration files")


are we expecting user to give both these flags to generate manifest files ? can't we just take single flag i.e the path they want to store the generated manifest and check from code that if length of this flag is >0 then user want to generate the manifest file
This'll ensure it's more user friendly as they would need to give single flag to generate manifest.

Also let's change the name of migration-output-dir to --upgraded-manifests-dir , this single flag would be necessary as this will clearly express the intentions of the flag

prakash100198 · 2025-04-17T12:20:38Z

 		success = false
 	}
+
+	if generateMigrationFiles {


one flag changes are done , check on len() instead of this flag

prakash100198 · 2025-04-17T12:27:46Z

 	Put(r ValidationResult) error
 	Flush() error
 	GetSummaryValidationResultBulk() []SummaryValidationResult
+	GenerateMigrationFiles(results []ValidationResult, outputDir string) error


Rename this func. to WriteUpgradedClusterManifests , this clearly shows func's intentions , GenerateMigrationFiles this is vague .

prakash100198 · 2025-04-17T12:38:14Z

+type MigrationFileGenerator struct {
+	outputDir string
+	noColor   bool
+}


change name to ManifestExporter
this can act as generic manifest generator, say for example if we want to generate manifest for old cluster version's data then I can use this struct and it's methods implemented on it . rn it's only for migrated files

prakash100198 · 2025-04-17T12:52:51Z

+		generator := pkg.NewMigrationFileGenerator(migrationOutputDir, noColor)
+		if err := generator.GenerateMigrationFiles(results); err != nil {


You should use outputManager.GenerateMigrationFiles (aftger renaming this func use renamed func name ) since you have already bound this func to OutputManager

prakash100198 · 2025-04-17T12:53:31Z

 	Deprecated             bool
 	LatestAPIVersion       string
 	IsVersionSupported     int
+	Resource               map[string]interface{} `json:"resource,omitempty"`


why is json tag necessary here ?

Also what will this key store ?

make the different struct ?

prakash100198 · 2025-04-17T12:58:12Z

+	for _, result := range results {
+		// Generate migration file for each resource, even if it has errors
+		if err := m.generateMigrationFile(result); err != nil {
+			return fmt.Errorf("failed to generate migration file for %s/%s: %v",


this is not optimal, this error printing in for loop will fill the whole terminal with errors which will also be unreadable , instead of this use struct to store error resp. for each errored result (gvk) and just continue generating manifest for other resources , for errored manifest just iterate over them and maybe generate another file with errored manifest output

let's disc this on monday I've few more pointers regarding this

prakash100198 · 2025-04-17T13:09:04Z

+		result.ResourceNamespace,
+		result.ResourceName,
+		strings.ToLower(result.Kind))


strings.ToLower is applied for kind , can we apply it for ResourceNamespace and ResourceName as well as general convention

prakash100198 · 2025-04-17T13:15:04Z

+}
+
+func (m *MigrationFileGenerator) GenerateMigrationFiles(results []ValidationResult) error {
+	if err := os.MkdirAll(m.outputDir, 0755); err != nil {


let's lower the permission level for this file , generally 755 permiossion is given to files or dirs which contains executables , since this only contains files to be read, lets make this 644

feat:cluster upgrade recommendation

76cb868

satyampsoni changed the title ~~feat:cluster upgrade recommendation~~ feat: cluster upgrade recommendation Apr 11, 2025

prakash100198 requested changes Apr 17, 2025

View reviewed changes

		RootCmd.Flags().BoolVar(&generateMigrationFiles, "generate-migration-files", false, "Generate migration files for API version updates")
		RootCmd.Flags().StringVar(&migrationOutputDir, "migration-output-dir", "migrations", "Directory to store generated migration files")

		generator := pkg.NewMigrationFileGenerator(migrationOutputDir, noColor)
		if err := generator.GenerateMigrationFiles(results); err != nil {

Uh oh!

Conversation

satyampsoni commented Apr 11, 2025

Add Migration File Generation Feature

Description

Changes

How it Works

Example Usage

Testing

Benefits

Related Issues

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants