-
Notifications
You must be signed in to change notification settings - Fork 28
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
Register operator #457
Register operator #457
Conversation
@@ -154,7 +154,7 @@ func (r *EndpointBindingPoller) createBinding(ctx context.Context, hashedName st | |||
return nil | |||
} | |||
|
|||
func (r *EndpointBindingPoller) updateBinding(ctx context.Context, binding *bindingsv1alpha1.EndpointBinding, apiEndpoint *EndpointBinding, urlBits *URLBits) error { | |||
func (r *EndpointBindingPoller) updateBinding(ctx context.Context, binding *bindingsv1alpha1.EndpointBinding, _ *EndpointBinding, urlBits *URLBits) error { |
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.
Heads up we might conflict on this change
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 can drop it out that commit easily if needed.
return r.controller.Reconcile(ctx, req, new(ngrokv1alpha1.KubernetesOperator)) | ||
} | ||
|
||
func (r *KubernetesOperatorReconciler) create(ctx context.Context, ko *ngrokv1alpha1.KubernetesOperator) error { |
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 see that the API registration is happening in the reconcile loop rather then on startup. I think this changes the expected behaviour of the ngrok-op where previously we would not want the container to ready up and we do not want bindingconfig to be reconciled unless the ngrok-op can claim/register with the API first.
I need to fix this before its ready. I thought it was but I discovered that we can't create the KubernetesOperator CR in the helm chart like I thought we could. I'll be moving this back to a similar pattern like we had above where it is created as part of the api-manger's main. |
79811aa
to
6009b17
Compare
It won't be used
When bindings are enabled, we need permission to manage the TLS secret used for communication with ngrok
6009b17
to
45881c5
Compare
45881c5
to
79dbe9b
Compare
What
Registers the the Kubernetes Operator with the ngrok API. If bindings are enabled, it generates the TLS secret with a private key, creates a CSR as part of registration, and saves the returned certificate in the secret.
How
Breaking Changes
Yes, some of the existing CRDs are being replaced with this change. We haven't released these yet, so unless someone is running on the latest
main
, they won't be impacted.