-
Notifications
You must be signed in to change notification settings - Fork 147
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
Postgres operator #678
base: master
Are you sure you want to change the base?
Postgres operator #678
Conversation
Signed-off-by: MdSahil-oss <[email protected]>
Signed-off-by: MdSahil-oss <[email protected]>
Signed-off-by: MdSahil-oss <[email protected]>
Signed-off-by: MdSahil-oss <[email protected]>
Signed-off-by: MdSahil-oss <[email protected]>
Signed-off-by: MdSahil-oss <[email protected]>
Signed-off-by: MdSahil-oss <[email protected]>
Signed-off-by: MdSahil-oss <[email protected]>
Signed-off-by: MdSahil-oss <[email protected]>
Signed-off-by: MdSahil-oss <[email protected]>
Signed-off-by: MdSahil-oss <[email protected]>
Signed-off-by: MdSahil-oss <[email protected]>
Signed-off-by: MdSahil-oss <[email protected]>
@wonderflow I've updated postgres addon, I hope now you will be able to run Bytebase with postgres addon :) |
Signed-off-by: MdSahil-oss <[email protected]>
@@ -0,0 +1,37 @@ | |||
"postgres-expose": { |
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.
can we reuse the expose trait?
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.
@wonderflow I've updated postgres-expose
trait as expose
trait. Please reivew it again.
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.
why do we need to create a new one instead of using expose
trait?
Signed-off-by: MdSahil-oss <[email protected]>
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.
Generally LGTM
apiVersion: "v1" | ||
kind: "Service" | ||
metadata: annotations: parameter.annotations | ||
spec: { | ||
selector: { | ||
"application": "spilo" | ||
"cluster-name": "postgres" | ||
"spilo-role": "master" | ||
"team": "acid" | ||
} |
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.
I had the same concern with @wonderflow . It seems that the existing Expose trait can accomplish the same functionality. Have you considered using it instead
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.
@wangyikewxgm @wonderflow I tried to do so, But I'm getting an error
could not create cluster: could not create master endpoint: endpoints "postgres" already exists
it seems like expose trait is getting created first then postgres cluster
using built in expose trait.
Signed-off-by: MdSahil-oss <[email protected]>
@wangyikewxgm @wonderflow I tried to do so, But I'm getting an error
it seems like expose trait is getting created first then postgres cluster using built in expose trait. |
@MdSahil-oss Hi, does this addon work? We can use postres-expose trait you added instead of built-in expose trait. |
Description of your changes
Update Postgres Addon.
How has this code been tested?
Checklist
I have:
[Addon]
,[example]
or[Doc]
).version
inmetadata.yaml
to generate a new version.Verified Addon promotion rules
If this pr wants to promote an experimental addon to verified, you must check whether meet these conditions too:
metadata.yaml
.README.md
.