-
Notifications
You must be signed in to change notification settings - Fork 462
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
[1.13] cli: Infer gloo deploy name #9719
Conversation
Issues linked to changelog: |
Co-authored-by: Nathan Fudenberg <[email protected]>
/kick Consul settings specify automatic detection of TLS services, but the rootCA reso |
client, err := KubeClient() | ||
if err != nil { | ||
errMessage := "error getting KubeClient" | ||
fmt.Println(errMessage) |
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.
nit: caller should be responsible for logging the error.
Fine with leaving it here but could we use contextutils.LoggerFrom
to get the logger?
for _, d := range deployments.Items { | ||
if d.Name != GlooDeploymentName { | ||
// At least 1 deployment exists, in case we dont find default update our error message | ||
errMessage = "too many app=gloo deployments, cannot decide which to target" |
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.
errMessage = "too many app=gloo deployments, cannot decide which to target" | |
errMessage = "too many deployments labeled gloo=gloo; cannot decide which to target" |
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.
Comments will be addressed in forward ports
@@ -27,6 +27,9 @@ var ( | |||
return fmt.Sprintf("Gloo has detected that the data plane is out of sync. The following types of resources have not been accepted: %v. "+ | |||
"Gloo will not be able to process any other configuration updates until these errors are resolved.", resourceNames) | |||
} | |||
|
|||
// Initialize the custom deployment name that is overwritten later on |
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.
nit:
// Initialize the custom deployment name that is overwritten later on | |
// Initialize the custom deployment name that is overwritten later on in the `CheckResources` function |
Description
Infer the gloo deployment name in cases where the deployment name is not the default
gloo
. The gloo deployment is identified by thegloo=gloo
label.This will be forward ported
Context
solo-io#9163
Interesting decisions
Rather than adding another flag for the gloo deployment name which can soon ballon into a flag for the name of each deployment, I've decided to identify the deployment via the labels.
Testing steps
The following commands depend on the gloo deployment name :
glooctl check
glooctl get proxy
glooctl proxy served-config
glooctl get upstream -o wide
Checklist: