-
Notifications
You must be signed in to change notification settings - Fork 59
feat(ws): add manifests for backend #455
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
feat(ws): add manifests for backend #455
Conversation
kind: Service | ||
metadata: | ||
name: nbv2-backend | ||
namespace: backend-system |
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.
app.kubernetes.io/component: backend | ||
app.kubernetes.io/managed-by: kustomize | ||
name: nbv2-backend | ||
namespace: backend-system |
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.
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.
w.r.t the "directory structure" for these files - I would propose the following:
workspaces/
├── backend/
├── api/
├── cmd/
├── internal/
├── manifests/
│ ├── kustomize/
│ │ ├── base/
│ │ ├── overlays/ # (not needed - including as merely an example of future-proofing)
│ │ │ ├── dev/
│ │ │ ├── staging/
│ │ │ └── prod/
│ │ └── options/ # (not needed - including as merely an example of future-proofing)
│ └── helm/ # (not needed - including as merely an example of future-proofing)
├── openapi/
├── Dockerfile
├── Makefile
├── README.md
├── ...
I feel this aligns more (loosely) with "industry standards" and/or expectations.
livenessProbe: | ||
httpGet: | ||
path: /api/v1/healthcheck | ||
port: 4000 |
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.
refer to name of port (as outlined in https://github.com/kubeflow/notebooks/pull/455/files#r2195457108)
5135cf3
to
6d78b09
Compare
cc9adad
to
69a6f97
Compare
@andyatmiami, I have made all the changes, and the PR is ready for a final review. |
/lgtm verified these changes with help of a script I have been working on to deploy Central Dashboard in |
workspaces/backend/manifests/kustomize/components/istio/destination-rule.yaml
Outdated
Show resolved
Hide resolved
workspaces/backend/manifests/kustomize/overlays/istio/kustomization.yaml
Outdated
Show resolved
Hide resolved
workspaces/backend/manifests/kustomize/components/istio/authorization-policy.yaml
Outdated
Show resolved
Hide resolved
workspaces/backend/manifests/kustomize/components/istio/authorization-policy.yaml
Outdated
Show resolved
Hide resolved
workspaces/backend/manifests/kustomize/components/common/kustomization.yaml
Outdated
Show resolved
Hide resolved
…#324 Signed-off-by: Liav Weiss (EXT-Nokia) <[email protected]>
…#324 Signed-off-by: Liav Weiss (EXT-Nokia) <[email protected]>
Signed-off-by: Liav Weiss (EXT-Nokia) <[email protected]>
…kubeflow#324 Signed-off-by: Liav Weiss (EXT-Nokia) <[email protected]>
69a6f97
to
52a38f8
Compare
/lgtm re-verified the end-to-end of running |
/ok-to-test |
Thanks @liavweiss and @andyatmiami, looks good to go for now. Next step is automated e2e testing. /lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: thesuperzapper The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
closes: #324
In this PR I have created the manifest workload for the backend component.
Added 2 targets to the Makefile
deploy
andundeploy
.How to deploy the backed component:
cd workspaces/backend
make docker-build
make docker-push
make deploy
kubectl port-forward svc/nbv2-backend 4000:4000 -n backend-system
How to undeploy the backed component:
cd workspaces/backend
make undeploy
@andyatmiami, regarding this PR, I want to consult with you:
NodePort
in theservice.yaml
mainly because we dont have yet an nginx-ingressI can change it to be type:
ClusterIP
if neededbackend
or we can leave it as it isnbv2-backend
?