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

feat(connector): create destination as soon as possible #3384

Merged
merged 2 commits into from
Oct 5, 2022

Conversation

mxyng
Copy link
Collaborator

@mxyng mxyng commented Oct 4, 2022

Summary

Update the connector to create itself as quickly as possible instead of waiting for a valid connection. This is mainly due to LoadBalancer potentially taking a significant amount of time to initialize. Creating the destination quickly allows the user to continue without waiting.

Update UI and CLI to output status == "Pending" when the destination is disconnected and does not have a connection URL. Omit pending and disconnected destinations when syncing kubeconfig

@mxyng mxyng force-pushed the mxyng/quick-destinations branch 2 times, most recently from b93b4c7 to 803a585 Compare October 4, 2022 02:03
@@ -297,23 +297,26 @@ func httpTransportFromOptions(opts ServerOptions) *http.Transport {
func syncDestination(con connector) error {
endpoint, err := getEndpointHostPort(con.k8s, con.options)
if err != nil {
return err
logging.L.Warn().Err(err).Msg("could not get host")
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe some more info here on why not getting the host could be happening would be useful, looks like something that is unexpected otherwise

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The err from getEndpointHostPort has more context which will be outputted through Err(err)

@@ -30,7 +30,7 @@ type DestinationConnection struct {

func (r DestinationConnection) ValidationRules() []validate.ValidationRule {
return []validate.ValidationRule{
validate.Required("url", r.URL),
validate.Required("ca", r.CA),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could this requirement break old connectors?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, current connection already consistently pass Connection.CA so this doesn't really do anything. Alternatively, I can remove the validation rules entirely.

internal/server/models/destination.go Outdated Show resolved Hide resolved
Update the connector to create itself as quickly as possible instead of
waiting for a valid connection. This is mainly due to LoadBalancer
potentially taking a significant amount of time to initialize. Creating
the destination quickly allows the user to continue without waiting.

Update UI and CLI to output status == "Pending" when the destination is
disconnected and does not have a connection URL.
Comment on lines +79 to +80
for _, uniqueID := range []string{"space", "moon", "maintain"} {
// set client.Headers so each destination becomes connected
Copy link
Contributor

Choose a reason for hiding this comment

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

I had this problem as well in #3391, I thought it might make sense to have CreateDestination mark it as "last seen at", like this: https://github.com/infrahq/infra/pull/3391/files#diff-487038165470a56e27ec73856c4cffb0b0808e9d00fd6a3481dd59d24f4e150c

@mxyng mxyng merged commit 225e6c3 into main Oct 5, 2022
@mxyng mxyng deleted the mxyng/quick-destinations branch October 5, 2022 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants