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

fix: ingress object meta value #144

Merged

Conversation

matthisholleville
Copy link
Contributor

@matthisholleville matthisholleville commented Mar 29, 2023

Closes #

πŸ“‘ Description

A merge request that includes the modifications made in arbreezy merge request available #143 and adds Ingress to the switch case GetParents.

βœ… Checks

  • My pull request adheres to the code style of this project
  • My code requires changes to the documentation
  • I have updated the documentation as required
  • All the tests have passed

β„Ή Additional Information

Aris Boutselis and others added 2 commits March 30, 2023 00:10
Signed-off-by: Aris Boutselis <aris.boutselis@senseon.io>
Signed-off-by: Matthis Holleville <matthish29@gmail.com>
@matthisholleville matthisholleville requested a review from a team as a code owner March 29, 2023 23:40
@matthisholleville matthisholleville changed the title Fix/ingress object meta value fix: ingress object meta value Mar 29, 2023
@arbreezy
Copy link
Member

@matthisholleville looks good, no worries.
I don't see the output of a missing secret when I analyze the Ingress object while having more errors for the same Ingress object, curious if you can replicate it as well.

@matthisholleville
Copy link
Contributor Author

matthisholleville commented Mar 29, 2023

Can you run : k8sgpt analyze --namespace my-namespace -o json | jq . please ?

This is my output :

...
{
  "kind": "Ingress",
  "name": "my-namespace/my-ingress",
  "error": [
    "Ingress uses the service my-namespace/my-ingress which does not exist.",
    "Ingress uses the secret my-namespace/my-missing-secret as a TLS certificate which does not exist."
  ],
  "details": "",
  "parentObject": "xxxxxx"
}

Do you have both errors?

@matthisholleville
Copy link
Contributor Author

@arbreezy You are right that if you don't use the JSON output format, you only get the first error (regardless of the analyzer). I don't think it's related to ingress exclusively. We should open an issue?

@arbreezy
Copy link
Member

Yes with json output it looks good and code is fine, not sure why plain text isn't showing it.
Is it working for you with both output formats?

@arbreezy
Copy link
Member

I already touched base on slack, going to have a proper look tomorrow as well, thanks for the prompt response @matthisholleville , most likely we need to open a new issue.

@matthisholleville
Copy link
Contributor Author

@arbreezy Indeed, only the first error from the array is displayed : https://github.com/k8sgpt-ai/k8sgpt/blob/main/cmd/analyze/analyze.go#L144

@AlexsJones AlexsJones mentioned this pull request Mar 30, 2023
4 tasks
@AlexsJones
Copy link
Member

Please check the bug fix I have proposed for the output and if it fixes your issue we can get this in also

@matthisholleville
Copy link
Contributor Author

All errors are displaying correctly with your fix. Great job!

@AlexsJones AlexsJones merged commit cf633b6 into k8sgpt-ai:main Mar 30, 2023
fyuan1316 pushed a commit to fyuan1316/k8sgpt that referenced this pull request Jun 26, 2023
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

None yet

4 participants