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

fix :area/helm charts: Add Notes.txt to .helmignore #221

Merged
merged 2 commits into from
Apr 21, 2024

Conversation

Sheikh-Abubaker
Copy link
Contributor

@Sheikh-Abubaker Sheikh-Abubaker commented Apr 21, 2024

Installing helm chart before adding Notes.txt to .helmignore

$ helm install my-cyclops cyclops-chart-0.3.0.tgz
Error: INSTALLATION FAILED: YAML parse error on cyclops-chart/templates/cyclops-ctrl/Notest.txt: error unmarshaling JSON: while decoding JSON: json: cannot unmarshal string into Go value of type releaseutil.SimpleHead

Installing helm chart after adding Notes.txt to .helmignore

helm install my-cyclops cyclops-chart-0.3.1.tgz
NAME: my-cyclops
LAST DEPLOYED: Sun Apr 21 17:50:49 2024
NAMESPACE: default
STATUS: deployed
REVISION: 1
TEST SUITE: None

$ kubectl get all 
-n cyclops
NAME                               READY   STATUS    RESTARTS   AGE
pod/cyclops-ctrl-f7598c8bb-fvppt   1/1     Running   0          14s
pod/cyclops-ui-55d897cb5-txp4j     1/1     Running   0          14s

NAME                   TYPE           CLUSTER-IP      EXTERNAL-IP   PORT(S)          AGE
service/cyclops-ctrl   ClusterIP      10.105.12.177   <none>        8080/TCP         14s
service/cyclops-ui     LoadBalancer   10.105.155.75   <pending>     3000:31384/TCP   14s

NAME                           READY   UP-TO-DATE   AVAILABLE   AGE
deployment.apps/cyclops-ctrl   1/1     1            1           14s
deployment.apps/cyclops-ui     1/1     1            1           14s

NAME                                     DESIRED   CURRENT   READY   AGE
replicaset.apps/cyclops-ctrl-f7598c8bb   1         1         1       14s
replicaset.apps/cyclops-ui-55d897cb5     1         1         1       14s

$ kubectl port-forward svc/cyclops-ui 3000:3000 -n cyclops
Forwarding from 127.0.0.1:3000 -> 80
Forwarding from [::1]:3000 -> 80
Handling connection for 3000
Handling connection for 3000

image

Signed-off-by: Sheikh-Abubaker <sheikhabubaker761@gmail.com>
@Sheikh-Abubaker Sheikh-Abubaker requested a review from a team as a code owner April 21, 2024 12:32
@Sheikh-Abubaker Sheikh-Abubaker requested review from petar-cvit and removed request for a team April 21, 2024 12:32
Copy link
Collaborator

@petar-cvit petar-cvit left a comment

Choose a reason for hiding this comment

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

Hey @Sheikh-Abubaker, thanks for the PR.

Seems like there was a misunderstanding. The issue is not with Cyclops's Helm chart; the issue is deploying other Helm charts through Cyclops. For instance, try connecting and deploying this chart:

repo: https://github.com/KaradzaJuraj/templates-pub
path: demo

You can see that in this chart, under the templates subdirectory, exists a Notes.txt that Cyclops is unable to parse.

If you try to deploy using the above chart, the UI will tell that the backend is unavailable:
Screenshot 2024-04-21 at 14 55 00

And if you go to the logs of the controller (the backend) you will find the following error => error unmarshaling JSON: while decoding JSON: json: cannot unmarshal string into Go value of type map[string]interface {}

You can tackle this issue in a separate PR, but since you updated the version of the chart, I would kindly ask you to update the image tags to v0.3.1 and appVersion to v0.3.1. You can update ctrl here and the UI here

@Sheikh-Abubaker
Copy link
Contributor Author

@petar-cvit my bad, let me look into this again, but I've a question do you want to move forward with these changes after I update the images tag mentioned above ?

@petar-cvit
Copy link
Collaborator

No worries; we appreciate the fact you picked it up in the first place.

Yeah, I'd say it would be best if you updated the versions in this PR and you opened a new one for the issue

@Sheikh-Abubaker
Copy link
Contributor Author

Yeah, I'd say it would be best if you updated the versions in this PR and you opened a new one for the issue

Thanks, let me update the tags and push the latest changes, while also editing the description since this PR does not solve #203 rather it can be an enhancement

Signed-off-by: Sheikh-Abubaker <sheikhabubaker761@gmail.com>
@Sheikh-Abubaker
Copy link
Contributor Author

@petar-cvit please checkout the latest changes and let me know it is up to mark.

Copy link
Collaborator

@petar-cvit petar-cvit left a comment

Choose a reason for hiding this comment

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

Thanks @Sheikh-Abubaker! Feel free to pick up #203. If you need any guidance reach out on GitHub or Discord

@petar-cvit petar-cvit merged commit f48a78b into cyclops-ui:main Apr 21, 2024
@Sheikh-Abubaker
Copy link
Contributor Author

Sheikh-Abubaker commented Apr 21, 2024

Thanks @Sheikh-Abubaker! Feel free to pick up #203. If you need any guidance reach out on GitHub or Discord

You're welcome @petar-cvit and thanks for your time too!!, I'll surely look into #203

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.

2 participants