-
Notifications
You must be signed in to change notification settings - Fork 21
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
Feature: Expose Devportal #969
Conversation
Signed-off-by: jasmingacic <jasmin.gacic@gmail.com>
✅ Deploy Preview for kusk-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
internal/controllers/parser.go
Outdated
//add devportal cluster | ||
hostPortPair, err := getUpstreamHost(&options.UpstreamOptions{ | ||
Service: &options.UpstreamService{ | ||
Name: "kusk-gateway-devportal", |
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.
We should get this from some config somewhere instead of hard coding it
I can't approve this through GH since it was me who opened this PR. |
I tested the setup with images i built from this PR it resulted in having this nice presentation For testing you can use this diff |
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.
Just some general feedback whilst I try it out.
reportError(err) | ||
return err | ||
} | ||
devPortalSpinner.Success("Installing Kusk Developer Portal") |
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.
Installing
to Installed
.
} | ||
|
||
kuskDevportalSpinner := utils.NewSpinner("Upgrading Kusk Devportal...") | ||
if err := applyf(filepath.Join(dir, manifests_dir, "devportal.yaml")); err != nil { |
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.
Minor point but devportal.yaml
is repeated.
- containerPort: 80 | ||
name: http | ||
protocol: TCP | ||
resources: |
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.
Are these strictly necessary? resources
limits, that is.
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.
For us, not really
but for a user who will install this, it's being a good citizen as it were in kubernetes
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.
LGTM.
I saw the logs arriving via a WebSocket to the dashboard.
Signed-off-by: jasmingacic jasmin.gacic@gmail.com
This PR addresses definition of DevPortal specs for backend as described here #654
closes #654
Changes