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

Rename directory in Tornjak Backend container #463

Merged
merged 3 commits into from
Aug 8, 2024

Conversation

vjesu5
Copy link
Contributor

@vjesu5 vjesu5 commented Jul 18, 2024

Changes proposed

This PR includes renaming the directory Tornjak from /opt/spire to /opt/tornjak mentioned in the opened issue #259 for:

  • Dockerfiles backend;
  • Shell script backend runner;
  • All of its references in the examples/ directory.

@vjesu5 vjesu5 changed the title Rename/tornjak directory Rename directory in Tornjak Backend container Jul 18, 2024
@vjesu5 vjesu5 force-pushed the rename/tornjak-directory branch from 2dca138 to a0d66bf Compare July 19, 2024 17:23
@vjesu5 vjesu5 marked this pull request as ready for review July 19, 2024 17:26
@maia-iyer maia-iyer linked an issue Jul 22, 2024 that may be closed by this pull request
@maia-iyer maia-iyer added the backend Tornjak API (Backend) label Jul 22, 2024
Copy link
Collaborator

@maia-iyer maia-iyer left a comment

Choose a reason for hiding this comment

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

Thanks for you contribution! I left a couple comments on the documentation - some of the commands you changed in the quickstart should be left alone because they are commands for a different container that DOES use the /opt/spire directory

Have you been able to build/test these changes?

@vjesu5 vjesu5 force-pushed the rename/tornjak-directory branch from a0d66bf to 6c09aa9 Compare July 23, 2024 12:47
@vjesu5
Copy link
Contributor Author

vjesu5 commented Jul 23, 2024

Hello, Maia.
i- Thanks for sharing the raised points, I just updated this PR by removing the directory update left in your comments.
ii- Regarding building the backend project: I successfully built the backend Dockerfiles for Tornjak. However, I cannot test it since the Contributing instructions for testing the backend in this repository look out-of-date. I am still unsure about this, let me know if I need to follow a different approach.
image
image

@vjesu5 vjesu5 requested a review from maia-iyer July 23, 2024 14:53
@maia-iyer
Copy link
Collaborator

That's fair! the backend documentation for testing an image does seem missing. To get started, I'm curious if you are able to run the quickstart? This has instructions for running tornjak locally. Once you are able to run it as is, you would be able to update the image in server-statefulset.yaml to the image you built, and test again

Definitely ask questions if any! I might try to update that documentation soon

@vjesu5
Copy link
Contributor Author

vjesu5 commented Jul 26, 2024

That's fair! the backend documentation for testing an image does seem missing. To get started, I'm curious if you are able to run the quickstart? This has instructions for running tornjak locally. Once you are able to run it as is, you would be able to update the image in server-statefulset.yaml to the image you built, and test again

Definitely ask questions if any! I might try to update that documentation soon

Thank you, Maia. 😄
After following the quickstart section, I successfully ran the Tornjak backend locally (also along with its UI) as is. However, going to test my local changes, I could not deploy the built image since the spire server is never ready (even waiting for some minutes and considering the image built finished successfully):


image

That is what I've added in the server-statefulset.yaml file based on the new image name:

image image

If possible, help me identify the issue. In parallel, I am trying to do other tests in a relentless quest to make this happen. 😄

@maia-iyer
Copy link
Collaborator

Thanks for the detailed debug info!

The local minikube cluster automatically attempts to pull from DockerHub. So when you specified the image in the yaml as tornjak-backend-vjesus-ubi:latest, it's trying to pull docker.io/tornjak-backend-vjesus-ubi:latest or something like that. I think if you run kubectl describe po -n spire spire-server-0 it would give more details towards the end of the output under the events section.

There's two possible remedies:

  1. Create a dockerhub account and push the image to dockerhub
  2. Simplest and quickets: I believe there's a minikube image load command that should be useful. I found this link with more useful information. Once this is done, you may need to delete the pod with a kubectl delete po -n spire spire-server-0 command to kick restart it

Let me know if this helps! If not, definitely show the outputs of kubectl describe po -n spire spire-server-0 and kubectl logs -n spire spire-server-0 -c tornjak-backend and we can debug further

@vjesu5
Copy link
Contributor Author

vjesu5 commented Jul 30, 2024

Thanks for the detailed debug info!

The local minikube cluster automatically attempts to pull from DockerHub. So when you specified the image in the yaml as tornjak-backend-vjesus-ubi:latest, it's trying to pull docker.io/tornjak-backend-vjesus-ubi:latest or something like that. I think if you run kubectl describe po -n spire spire-server-0 it would give more details towards the end of the output under the events section.

There's two possible remedies:

  1. Create a dockerhub account and push the image to dockerhub
  2. Simplest and quickets: I believe there's a minikube image load command that should be useful. I found this link with more useful information. Once this is done, you may need to delete the pod with a kubectl delete po -n spire spire-server-0 command to kick restart it

Let me know if this helps! If not, definitely show the outputs of kubectl describe po -n spire spire-server-0 and kubectl logs -n spire spire-server-0 -c tornjak-backend and we can debug further

Thank you, Maia! It completely helped me.
My misunderstanding was precisely that Minikube automatically pulls the image from the repository (despite the possibility of adjusting and forcing it to use the local image). After publishing the image to the hub, I built and ran the Tornjak backend successfully.

So now I can run the spire server, along with the tornjak backend (with the changes), and its frontend.

image image image

In complement, I tested with the non-ubi image which also seem to work fine.

image

Both UBI and non-UBI images were published on https://hub.docker.com/r/vjesu5/tornjak-backend-ubi and https://hub.docker.com/r/vjesu5/tornjak-backend respectively.


I've just tested this end-to-end by navigating through the UI application. I'm unsure how effective the test was to ensure that my changes (path renames) didn't break the application behavior.

If you have any additional concerns or backend prints you may require, please let me know.

This Pull Request description was updated based on what was changed in fact.

@maia-iyer maia-iyer added this to the 1.8.0 milestone Aug 6, 2024
Copy link
Collaborator

@maia-iyer maia-iyer left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you very much for the contribution!

@maia-iyer maia-iyer merged commit cf4a398 into spiffe:dev Aug 8, 2024
2 checks passed
@vjesu5 vjesu5 deleted the rename/tornjak-directory branch August 8, 2024 20:56
maia-iyer pushed a commit that referenced this pull request Sep 17, 2024
* refactor: updates the tornjak directory for backend dockerfiles

Signed-off-by: Vanessa de Jesus Silva <[email protected]>

* refactor: updates the tornjak directory references for backend runner script

Signed-off-by: Vanessa de Jesus Silva <[email protected]>

* refactor: updates the tornjak directory references in examples/ dir

Signed-off-by: Vanessa de Jesus Silva <[email protected]>

---------

Signed-off-by: Vanessa de Jesus Silva <[email protected]>
maia-iyer pushed a commit that referenced this pull request Sep 17, 2024
* refactor: updates the tornjak directory for backend dockerfiles

Signed-off-by: Vanessa de Jesus Silva <[email protected]>

* refactor: updates the tornjak directory references for backend runner script

Signed-off-by: Vanessa de Jesus Silva <[email protected]>

* refactor: updates the tornjak directory references in examples/ dir

Signed-off-by: Vanessa de Jesus Silva <[email protected]>

---------

Signed-off-by: Vanessa de Jesus Silva Sarmento <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Tornjak API (Backend)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rename directory in Tornjak Backend container
2 participants