Skip to content

cilium-cli status: fail fast on terminal error #35048

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 21, 2024
Merged

cilium-cli status: fail fast on terminal error #35048

merged 1 commit into from
Oct 21, 2024

Conversation

nimishamehta5
Copy link
Contributor

@nimishamehta5 nimishamehta5 commented Sep 26, 2024 •
edited
Loading

Please ensure your pull request adheres to the following guidelines:

  • For first time contributors, read Submitting a pull request
  • All code is covered by unit and/or runtime tests where feasible.
  • All commits contain a well written commit description including a title,
    description and a Fixes: #XXX line if the commit addresses a particular
    GitHub issue.
  • If your commit description contains a Fixes: <commit-id> tag, then
    please add the commit author[s] as reviewer[s] to this issue.
  • All commits are signed off. See the section Developer’s Certificate of Origin
  • Provide a title or release-note blurb suitable for the release notes.
  • Are you a user of Cilium? Please add yourself to the Users doc
  • Thanks for contributing!

Fixes: #34537

  • Allow the cilium status --wait command to fail fast when there is a terminal error and not continue to retry.
  • Allow collection of previous cilium agent container logs when there are restarts.

Testing -

Intentionally introduce a panic in the cilium agent to induce CrashLoopBackOff state.
Build and run cilium status --wait, it fails fast with the logs printed in the output:

    /¯¯\
 /¯¯\__/¯¯\    Cilium:             3 errors
 \__/¯¯\__/    Operator:           OK
 /¯¯\__/¯¯\    Envoy DaemonSet:    OK
 \__/¯¯\__/    Hubble Relay:       disabled
    \__/       ClusterMesh:        disabled

DaemonSet              cilium             Desired: 2, Unavailable: 2/2
DaemonSet              cilium-envoy       Desired: 2, Ready: 2/2, Available: 2/2
Deployment             cilium-operator    Desired: 1, Ready: 1/1, Available: 1/1
Containers:            cilium             Running: 2
                       cilium-envoy       Running: 2
                       cilium-operator    Running: 1
Cluster Pods:          0/0 managed by Cilium
Helm chart version:    1.17.0-dev
Image versions         cilium             localhost:5000/cilium/cilium-dev:local: 2
                       cilium-envoy       quay.io/cilium/cilium-envoy:v1.30.4-1725856146-ae435a5bef3856f4de98fa360ecfa6a0f5b0f7a1@sha256:f9c2a725a702d9fe4a4e038588a79e72955cf46c789914f940d906d65e92527e: 2
                       cilium-operator    localhost:5000/cilium/operator-generic:local: 1
Errors:                cilium             cilium          2 pods of DaemonSet cilium are not ready
                       cilium             cilium-dr4ql    unable to retrieve cilium status: container cilium-agent is in CrashLoopBackOff, exited with code 2: time="2024-09-26T04:55:38.054243877Z" level=debug msg="Skipped reading configuration file" error="Config File \"cilium\" Not Found in \"[/root]\"" subsys=config
time="2024-09-26T04:55:38.05457721Z" level=info msg="Memory available for map entries (0.003% of 4108660736B): 10271651B" subsys=config
time="2024-09-26T04:55:38.054587502Z" level=debug msg="Total memory for default map entries: 149422080" subsys=config
panic: Inducing crashloopbackoff

goroutine 1 [running]:
github.com/cilium/cilium/pkg/option.(*DaemonConfig).calculateDynamicBPFMapSizes(0x6bbde60, 0x3d2f8eb?, 0xf4e53000, 0x56d200?)
    /go/src/github.com/cilium/cilium/pkg/option/config.go:3928 +0x140
github.com/cilium/cilium/pkg/option.(*DaemonConfig).calculateBPFMapSizes(0x6bbde60, 0x40006ed6c0)
    /go/src/github.com/cilium/cilium/pkg/option/config.go:3879 +0x344
github.com/cilium/cilium/pkg/option.(*DaemonConfig).Populate(0x6bbde60, 0x40006ed6c0)
    /go/src/github.com/cilium/cilium/pkg/option/config.go:3298 +0x26e4
github.com/cilium/cilium/daemon/cmd.initDaemonConfig(0x40006ed6c0)
    /go/src/github.com/cilium/cilium/daemon/cmd/daemon_main.go:1206 +0x54
github.com/cilium/cilium/daemon/cmd.NewAgentCmd.func3()
    /go/src/github.com/cilium/cilium/daemon/cmd/root.go:72 +0x24
github.com/spf13/cobra.(*Command).preRun(...)
    /go/src/github.com/cilium/cilium/vendor/github.com/spf13/cobra/command.go:1019
github.com/spf13/cobra.(*Command).execute(0x4000760008, {0x4000140010, 0x1, 0x1})
    /go/src/github.com/cilium/cilium/vendor/github.com/spf13/cobra/command.go:929 +0x424
github.com/spf13/cobra.(*Command).ExecuteC(0x4000760008)
    /go/src/github.com/cilium/cilium/vendor/github.com/spf13/cobra/command.go:1117 +0x344
github.com/spf13/cobra.(*Command).Execute(...)
    /go/src/github.com/cilium/cil
    cilium    cilium-ndpqt    unable to retrieve cilium status: container cilium-agent is in CrashLoopBackOff, exited with code 2: time="2024-09-26T04:55:57.162269969Z" level=debug msg="Skipped reading configuration file" error="Config File \"cilium\" Not Found in \"[/root]\"" subsys=config
time="2024-09-26T04:55:57.162573178Z" level=info msg="Memory available for map entries (0.003% of 4108660736B): 10271651B" subsys=config
time="2024-09-26T04:55:57.162586803Z" level=debug msg="Total memory for default map entries: 149422080" subsys=config
panic: Inducing crashloopbackoff

@nimishamehta5 nimishamehta5 requested review from a team as code owners September 26, 2024 05:24
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Sep 26, 2024
@github-actions github-actions bot added cilium-cli This PR contains changes related with cilium-cli cilium-cli-exclusive This PR only impacts cilium-cli binary kind/community-contribution This was a contribution made by a community member. labels Sep 26, 2024
@nimishamehta5 nimishamehta5 force-pushed the 34537/cilium-cli-status-logs branch from ec67cb4 to 3352b79 Compare September 26, 2024 05:25
giorio94
giorio94 approved these changes Sep 26, 2024
View reviewed changes
Copy link
Member

@giorio94 giorio94 left a comment

Choose a reason for hiding this comment

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

/lgtm for the clustermesh changes.

@giorio94 giorio94 added the release-note/minor This PR changes functionality that users may find relevant to operating Cilium. label Sep 26, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Sep 26, 2024
@giorio94
Copy link
Member

/test

derailed
derailed requested changes Oct 1, 2024
View reviewed changes
Copy link
Contributor

@derailed derailed left a comment

Choose a reason for hiding this comment

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

@nimishamehta5 Good catch!

@nimishamehta5 nimishamehta5 force-pushed the 34537/cilium-cli-status-logs branch from 3352b79 to 66affcd Compare October 1, 2024 16:59
@nimishamehta5 nimishamehta5 requested a review from derailed October 1, 2024 16:59
derailed
derailed approved these changes Oct 3, 2024
View reviewed changes
Copy link
Contributor

@derailed derailed left a comment

Choose a reason for hiding this comment

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

@nimishamehta5 Thanks for the updates!

derailed
derailed reviewed Oct 3, 2024
View reviewed changes
@nimishamehta5 nimishamehta5 force-pushed the 34537/cilium-cli-status-logs branch from 66affcd to 81e584f Compare October 3, 2024 03:35
@nimishamehta5
Copy link
Contributor Author

@youngnick: do you mind reviewing this small PR? thanks!

@aanm
Copy link
Member

aanm commented Oct 11, 2024

/test

@aanm aanm enabled auto-merge October 11, 2024 10:36
@chaunceyjiang
Copy link
Member

/ci-e2e-upgrade

@nimishamehta5 @aanm
cilium-cli status: fail fast on terminal error
0760636 
Fixes: cilium#34537

Signed-off-by: Nimisha Mehta <nimishamehta5@gmail.com>
@aanm aanm force-pushed the 34537/cilium-cli-status-logs branch from 81e584f to 0760636 Compare October 21, 2024 09:43
@aanm
Copy link
Member

aanm commented Oct 21, 2024

/test

@aanm aanm added this pull request to the merge queue Oct 21, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Oct 21, 2024
Merged via the queue into cilium:main with commit 3dbad77 Oct 21, 2024
62 checks passed
@aanm aanm mentioned this pull request Oct 24, 2024
Closed
@github-actions github-actions bot mentioned this pull request Nov 11, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@derailed derailed derailed approved these changes

@aanm aanm aanm approved these changes

@youngnick youngnick youngnick approved these changes

@giorio94 giorio94 giorio94 approved these changes

Assignees
No one assigned
Labels
cilium-cli This PR contains changes related with cilium-cli cilium-cli-exclusive This PR only impacts cilium-cli binary kind/community-contribution This was a contribution made by a community member. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/minor This PR changes functionality that users may find relevant to operating Cilium.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update the smoke tests to fail more quickly and more clearly when there is a fatal error in the agent
6 participants
@nimishamehta5 @giorio94 @aanm @chaunceyjiang @derailed @youngnick