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

skip creation later to improve visibility of errors #1013

Merged
merged 3 commits into from
Jun 17, 2020

Conversation

FxKu
Copy link
Member

@FxKu FxKu commented Jun 11, 2020

if the cluster name does not start with the teamId we are currently skipping the add event, which makes it hard to track the error from the UI. It is emitted neither to worker logs nor to K8s events. The PostgresClusterStatus, although set during marshaling, remains empty.

The PR does not skip the addEvent but will process it until the cluster would get created. This allows to show the error in logs accessible from UI and to record a K8s event with a working ObjectReference.

@@ -156,6 +159,34 @@ func NewFromConfig(cfg *rest.Config) (KubernetesClient, error) {
return kubeClient, nil
}

// SetStatus of Postgres cluster
// TODO: eventually switch to updateStatus() for kubernetes 1.11 and above
Copy link
Member

Choose a reason for hiding this comment

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

Whats with this comment?

Copy link
Member Author

Choose a reason for hiding this comment

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

traveled with the code. I'll remove it. UpdateStatus requires a Postgresql struct with updated status. Seems easier as of now to just patch the CRD.

Copy link
Member Author

Choose a reason for hiding this comment

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

I wonder if SetStatus should be called differently now that it lives inside k8sutil. Maybe SetPgStatus or SetCRDStatus

@Jan-M
Copy link
Member

Jan-M commented Jun 17, 2020

👍

1 similar comment
@FxKu
Copy link
Member Author

FxKu commented Jun 17, 2020

👍

@FxKu FxKu merged commit 0c6655a into master Jun 17, 2020
@FxKu FxKu added this to the 1.6 milestone Dec 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants