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

added dry-run flag to create core_instance kubernetes #346

Merged
merged 4 commits into from
Feb 24, 2023

Conversation

jasmingacic
Copy link
Contributor

Summary of this proposal

Fixes https://github.com/calyptia/cloud/issues/767

now by running:

# calyptia create core_instance kubernetes --dry-run
[Debug] [Error] could not retrive your stored url: data not found
The default url https://cloud-api.calyptia.com will be used instead

---
kind: Secret
apiVersion: v1
metadata:
  name: calyptia-trusted-ink-155e-default-secret
  creationTimestamp: null
  labels:
    app.kubernetes.io/created-by: calyptia-cli
    app.kubernetes.io/managed-by: calyptia-cli
    app.kubernetes.io/part-of: calyptia
...

users can get k8s yaml file with k8s resources the output can be piped like

# calyptia create core_instance kubernetes --dry-run | kubectl apply -f -
[Debug] [Error] could not retrive your stored url: data not found
The default url https://cloud-api.calyptia.com/ will be used instead

secret/calyptia-clean-darkhawk-2b22-default-secret created
clusterrole.rbac.authorization.k8s.io/calyptia-clean-darkhawk-2b22-default-cluster-role created
serviceaccount/calyptia-clean-darkhawk-2b22-default-service-account created
clusterrolebinding.rbac.authorization.k8s.io/calyptia-clean-darkhawk-2b22-default-cluster-role-binding created
deployment.apps/calyptia-clean-darkhawk-2b22-default-deployment created

Signed-off-by: jasmingacic <jasmin@calyptia.com>
ObjectMeta: metadata,
Data: map[string][]byte{
metadata.Name: agg.PrivateRSAKey,
},
}, metav1.CreateOptions{})
}
req.TypeMeta = metav1.TypeMeta{
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These values need to be populated so the yaml output contains valid values.

@@ -166,3 +182,14 @@ func newCmdCreateCoreInstanceOnK8s(config *cfg.Config, testClientSet kubernetes.

return cmd
}

// GetK8sYaml strips empty properties which would typically be marshalled with yaml.Marshal
func GetK8sYaml(req interface{}) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If return object from k8s is marshalled into Yaml all empty fields will be marshalled as well. Only with JSON that doesn't happen.
So this is a clumsy workaround that.

@patrick-stephens
Copy link
Contributor

Does this only work for that specific command? I can see it being useful for any interacting directly with K8S but we probably need to consider the AWS/GCP command to ensure it fails there for now.

We don't want a user thinking they'll get a dry run but actually stuff is applied.

@jasmingacic
Copy link
Contributor Author

jasmingacic commented Feb 23, 2023

Dry run is only for Kubernetes I didn't touch GCP or AWS. I don't think we can make it like this for GCP and AWS. We could maybe print out what the command will do but it won't be something we can pipe into kubectl.
Perhaps making curl output could be useful?

@@ -166,3 +182,14 @@ func newCmdCreateCoreInstanceOnK8s(config *cfg.Config, testClientSet kubernetes.

return cmd
}

// GetK8sYaml strips empty properties which would typically be marshalled with yaml.Marshal
func GetK8sYaml(req interface{}) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
func GetK8sYaml(req interface{}) {
func printK8sYaml(req any) {

"Get" denotes getting a value, but this function doesn't return anything. Also, it's not reused outside so why export it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

input := strings.NewReader(string(out))
var output strings.Builder
if err := json2yaml.Convert(&output, input); err != nil {
log.Fatalln(err)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not fan of panic... Maybe you can log.Print and return instead. Panic will give you a stack trace.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

@nicolasparada nicolasparada left a comment

Choose a reason for hiding this comment

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

Left some comments.

Signed-off-by: jasmingacic <jasmin@calyptia.com>
input := strings.NewReader(string(out))
var output strings.Builder
if err := json2yaml.Convert(&output, input); err != nil {
log.Println("failed to convert JSON to YAML:", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
log.Println("failed to convert JSON to YAML:", err)
log.Println("failed to convert JSON to YAML:", err)
return

Copy link
Contributor

@nicolasparada nicolasparada left a comment

Choose a reason for hiding this comment

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

Missing a return.

Signed-off-by: jasmingacic <jasmin@calyptia.com>
@sonarcloud
Copy link

sonarcloud bot commented Feb 24, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@patrick-stephens
Copy link
Contributor

LGTM - I'll merge once CI is green

@patrick-stephens patrick-stephens merged commit e63169a into completion_refactor Feb 24, 2023
@patrick-stephens patrick-stephens deleted the cli_dry-run_for_k8s branch February 24, 2023 16:54
patrick-stephens pushed a commit that referenced this pull request Feb 24, 2023
* formatters and completer

* added pkg/config package

Signed-off-by: jasmingacic <jasmin@calyptia.com>

* minor cleanup

* part of completer done

* another completer phase

* another completer phase 3

* removed PKG directory

Signed-off-by: jasmingacic <jasmin@calyptia.com>

* moved commands to model directories
changed goreleaser, added Makefile
and various changes

Signed-off-by: jasmingacic <jasmin@calyptia.com>

* added install.sh - it fetches CLI binaries from GH

Signed-off-by: jasmingacic <jasmin@calyptia.com>

* build fix

Signed-off-by: jasmingacic <jasmin@calyptia.com>

* minor tweak

Signed-off-by: jasmingacic <jasmin@calyptia.com>

* Dockerfile fix

Signed-off-by: jasmingacic <jasmin.gacic@gmail.com>

* fixed install.sh

Signed-off-by: jasmingacic <jasmin.gacic@gmail.com>

* check config fix

Signed-off-by: jasmingacic <jasmin.gacic@gmail.com>

* shellcheck fixes for install.sh

Signed-off-by: jasmingacic <jasmin@calyptia.com>

* added slice package

Signed-off-by: jasmingacic <jasmin@calyptia.com>

* renamed UniqueSlice to Unique

Signed-off-by: jasmingacic <jasmin@calyptia.com>

* moved formatting functions to formatters package

Signed-off-by: jasmingacic <jasmin@calyptia.com>

* added confirm package

Signed-off-by: jasmingacic <jasmin@calyptia.com>

* added package metric

Signed-off-by: jasmingacic <jasmin@calyptia.com>

* rename ReadConfirm to Read

Signed-off-by: jasmingacic <jasmin@calyptia.com>

* Added calyptia version subcommand

Signed-off-by: jasmingacic <jasmin@calyptia.com>

* switched from fmt to cmd

Signed-off-by: jasmingacic <jasmin@calyptia.com>

* Added short description

* removed error silencing

Signed-off-by: jasmingacic <jasmin@calyptia.com>

* typo fix

Signed-off-by: jasmingacic <jasmin@calyptia.com>

* typo fix and default BaseURL

Signed-off-by: jasmingacic <jasmin@calyptia.com>

* error handling for token retrieval

Signed-off-by: jasmingacic <jasmin@calyptia.com>

* adding pod namespace as environment variable in the container (#349)

* adding pod namespace as environment variable in the container

Signed-off-by: jasmingacic <jasmin@calyptia.com>

* added deployment name to the container

Signed-off-by: jasmingacic <jasmin@calyptia.com>

---------

Signed-off-by: jasmingacic <jasmin@calyptia.com>

* added dry-run flag to create core_instance kubernetes (#346)

* added dry-run flag to create core_instance kubernetes

Signed-off-by: jasmingacic <jasmin@calyptia.com>

* cmd: check base url from localdata error

* goimports

* remove unused coreInstanceKeys func

* re-sort dangling import groups

---------

Signed-off-by: jasmingacic <jasmin@calyptia.com>
Signed-off-by: jasmingacic <jasmin.gacic@gmail.com>
Co-authored-by: Nicolás Parada <nicolas@calyptia.com>
gabrielbussolo pushed a commit that referenced this pull request Mar 1, 2023
* formatters and completer

* added pkg/config package

Signed-off-by: jasmingacic <jasmin@calyptia.com>

* minor cleanup

* part of completer done

* another completer phase

* another completer phase 3

* removed PKG directory

Signed-off-by: jasmingacic <jasmin@calyptia.com>

* moved commands to model directories
changed goreleaser, added Makefile
and various changes

Signed-off-by: jasmingacic <jasmin@calyptia.com>

* added install.sh - it fetches CLI binaries from GH

Signed-off-by: jasmingacic <jasmin@calyptia.com>

* build fix

Signed-off-by: jasmingacic <jasmin@calyptia.com>

* minor tweak

Signed-off-by: jasmingacic <jasmin@calyptia.com>

* Dockerfile fix

Signed-off-by: jasmingacic <jasmin.gacic@gmail.com>

* fixed install.sh

Signed-off-by: jasmingacic <jasmin.gacic@gmail.com>

* check config fix

Signed-off-by: jasmingacic <jasmin.gacic@gmail.com>

* shellcheck fixes for install.sh

Signed-off-by: jasmingacic <jasmin@calyptia.com>

* added slice package

Signed-off-by: jasmingacic <jasmin@calyptia.com>

* renamed UniqueSlice to Unique

Signed-off-by: jasmingacic <jasmin@calyptia.com>

* moved formatting functions to formatters package

Signed-off-by: jasmingacic <jasmin@calyptia.com>

* added confirm package

Signed-off-by: jasmingacic <jasmin@calyptia.com>

* added package metric

Signed-off-by: jasmingacic <jasmin@calyptia.com>

* rename ReadConfirm to Read

Signed-off-by: jasmingacic <jasmin@calyptia.com>

* Added calyptia version subcommand

Signed-off-by: jasmingacic <jasmin@calyptia.com>

* switched from fmt to cmd

Signed-off-by: jasmingacic <jasmin@calyptia.com>

* Added short description

* removed error silencing

Signed-off-by: jasmingacic <jasmin@calyptia.com>

* typo fix

Signed-off-by: jasmingacic <jasmin@calyptia.com>

* typo fix and default BaseURL

Signed-off-by: jasmingacic <jasmin@calyptia.com>

* error handling for token retrieval

Signed-off-by: jasmingacic <jasmin@calyptia.com>

* adding pod namespace as environment variable in the container (#349)

* adding pod namespace as environment variable in the container

Signed-off-by: jasmingacic <jasmin@calyptia.com>

* added deployment name to the container

Signed-off-by: jasmingacic <jasmin@calyptia.com>

---------

Signed-off-by: jasmingacic <jasmin@calyptia.com>

* added dry-run flag to create core_instance kubernetes (#346)

* added dry-run flag to create core_instance kubernetes

Signed-off-by: jasmingacic <jasmin@calyptia.com>

* cmd: check base url from localdata error

* goimports

* remove unused coreInstanceKeys func

* re-sort dangling import groups

---------

Signed-off-by: jasmingacic <jasmin@calyptia.com>
Signed-off-by: jasmingacic <jasmin.gacic@gmail.com>
Co-authored-by: Nicolás Parada <nicolas@calyptia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants