-
Notifications
You must be signed in to change notification settings - Fork 772
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
cyclops cli: support for update command #475
cyclops cli: support for update command #475
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @ashish111333, left two comments
cyctl/internal/update/modules.go
Outdated
|
||
// updates the given module from cyclops API | ||
// currently supports updating replica count for a module | ||
func updateModule(clientset *client.CyclopsV1Alpha1Client, moduleName, key string, value int) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you make the value type interface
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
minor fix
minor fix
minor fix
minor fix
have done the requested changes,can you review it now @petar-cvit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @ashish111333, left a couple of comments
cyctl/internal/update/modules.go
Outdated
return | ||
|
||
} | ||
SpecValuesMap := make(map[string]interface{}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rename to specValuesMap
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
cyctl/internal/update/modules.go
Outdated
return | ||
|
||
} | ||
err = unstructured.SetNestedField(SpecValuesMap, value, key) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you have a key that is nested (like scaling.replicas
) you will have to split the keys by dots
err = unstructured.SetNestedField(SpecValuesMap, value, key) | |
err = unstructured.SetNestedField(SpecValuesMap, value, strings.Split(key, ".")...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
fmt.Println("failed to encode to json: ", err) | ||
return | ||
} | ||
module.Spec.Values = apiextensionv1.JSON{Raw: updatedSpecValues} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You will also have to populate typeMeta before updating
module.Spec.Values = apiextensionv1.JSON{Raw: updatedSpecValues} | |
module.Spec.Values = apiextensionv1.JSON{Raw: updatedSpecValues} | |
module.TypeMeta = v1.TypeMeta{ | |
APIVersion: "cyclops-ui.com/v1alpha1", | |
Kind: "Module", | |
} |
cyctl/internal/update/modules.go
Outdated
fmt.Println("failed to update module: ", err) | ||
return | ||
} | ||
fmt.Printf("successfully updated %v", updatedModule.Name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you use moduleName without getting it from the updatedModule
fmt.Printf("successfully updated %v", updatedModule.Name) | |
fmt.Printf("successfully updated %v", moduleName) |
minor fixes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ashish111333 Build is failing because you need to unpack split output using strings.Split(key, ".")...
. Also, remove unused updatedModule
variable
cyctl/internal/update/modules.go
Outdated
} | ||
|
||
var ( | ||
UpdateModule = &cobra.Command{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you update the name to updateModuleCMD
. Update command name in init func as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah
build error fix
minor fix
minor fix
minor fix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @ashish111333 π§‘
closes #454
π Description
adds support for the updating modules using cyctl cli
β Checks
Additional context
cyclops cli currently supports create,delete,describe,get ,this PR adds support for update