-
Notifications
You must be signed in to change notification settings - Fork 1
increase timeout and added some log lines #197
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
base: main
Are you sure you want to change the base?
Conversation
loiro
left a comment
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.
LGTM
| { | ||
| String labelSelector = String.format("%s=%s", serviceLabel, service); | ||
| log.debug("Filtering deployment {} using selector {}", resource, labelSelector); | ||
| log.debug("Filtering resource {} using selector {}", resource, labelSelector); |
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 think we could keep it as Filtering deployment for consistency with other logs and because we actually filtering data from deployments....
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 but it's logging like this Filtering deployment DEPLOYMENT using selector. Actually argument KubernetesResources resource is representing the resource type which in this function is Deployment.
There are another resource type defined : https://github.com/freenowtech/sauron/blob/main/plugins/kubernetesapi-report/src/main/java/com/freenow/sauron/plugins/utils/KubernetesResources.java#L5
| log.error("getDeploymentSpec failed '{}'", ex.getMessage(), ex); | ||
| } | ||
|
|
||
| log.warn("No deployment returned for service '{}'", service); |
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.
Should we consider this log as DEBUG instead? Also, maybe add the labelSelector?
| public static final String K8S_DEFAULT_NAMESPACE = "default"; | ||
|
|
||
| public static final int K8S_API_TIMEOUT_SECONDS = 5; | ||
| public static final int K8S_API_TIMEOUT_SECONDS = 30; |
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’m not sure if you noticed, but the timeout is only happening in the mgmt. Are we sure that the K8s API is accessible there?
No description provided.