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

[v1.14] Add SPIRE connection to status #27649

Merged
merged 1 commit into from Sep 5, 2023

Conversation

meyskens
Copy link
Member

@meyskens meyskens commented Aug 23, 2023

This is a backport of #26896

The automatic backport failed in #27629

Add SPIRE connection to `cilium status`
$ for pr in 26896; do contrib/backporting/set-labels.py $pr done 1.14; done

@meyskens meyskens requested review from a team as code owners August 23, 2023 11:14
@meyskens meyskens requested a review from aanm August 23, 2023 11:14
@maintainer-s-little-helper maintainer-s-little-helper bot added backport/1.14 This PR represents a backport for Cilium 1.14.x of a PR that was merged to main. kind/backports This PR provides functionality previously merged into master. labels Aug 23, 2023
@meyskens
Copy link
Member Author

/test-backport-1.14

@meyskens meyskens force-pushed the meyskens/backport-spire-status branch from ea43f35 to ba957cf Compare August 23, 2023 12:19
@meyskens
Copy link
Member Author

/test-backport-1.14

@meyskens meyskens requested a review from tklauser August 23, 2023 13:43
@joestringer
Copy link
Member

Please consider using contrib/backporting/cherry-pick to auto-insert the [ upstream commit xxxxxxxxx ] text into cherry-picked commits. This helps reviewers and future readers for the backported changes by simplifying the method to compare the backport vs. original change, and to trace back to the original change for the full context, discussion, target releases and so on.

@joestringer
Copy link
Member

joestringer commented Aug 23, 2023

Backports performed the usual way will also have automatic release note correlation to the original PR. More specifically, it needs text like this in the PR description:

```upstream-prs
$ for pr in 26896; do contrib/backporting/set-labels.py $pr done 1.14; done
```

@joestringer
Copy link
Member

joestringer commented Aug 23, 2023

The automatic backport failed in #27629

Please consider detailing what these problems were for reviewers so that we can assess whether the differences between the PRs may materially affect the function of the PR now that it's backported, and so we can focus review on the risk areas for integration with older branches.

Ideally this detail is included in the commit messages so that this state is stored with the code. It can also be helpful to include this detail in the PR description.

@meyskens meyskens force-pushed the meyskens/backport-spire-status branch from ba957cf to 7ccd9c3 Compare August 24, 2023 09:35
@tklauser
Copy link
Member

/test-backport-1.14

@meyskens meyskens force-pushed the meyskens/backport-spire-status branch from 7ccd9c3 to 2ebccaa Compare August 25, 2023 08:31
@meyskens
Copy link
Member Author

/test-backport-1.14

Copy link
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

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

Changes LGTM (I compared the original commit to this backported version side-by-side), but the commit message confused me. Is there something related to ipcache that we should be reviewing more closely?

api/v1/models/status_response.go Show resolved Hide resolved
@maintainer-s-little-helper maintainer-s-little-helper bot added ready-to-merge This PR has passed all tests and received consensus from code owners to merge. labels Aug 25, 2023
@meyskens meyskens force-pushed the meyskens/backport-spire-status branch from 2ebccaa to 838c8eb Compare August 28, 2023 07:56
@meyskens
Copy link
Member Author

/test-backport-1.14

@meyskens meyskens force-pushed the meyskens/backport-spire-status branch from 838c8eb to f2f57ca Compare August 28, 2023 08:56
@aditighag
Copy link
Member

@meyskens Err... Looks like MLH added the read-to-merge label prematurely. Can you check if all the required tests passed? Please re-add the label once the PR is ready for merge.

@aditighag aditighag removed the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Sep 1, 2023
[ upstream commit 1410a66 ]

This adds the SPIRE connection to cilium status, this then can be used
by the CLI tool to surface errors and/or wait for SPIRE to be ready.
If Auth is disabled it will surface the disabled status.

This commit is a manual backport due to formatting and import conflicts
between main and v1.14 due to the replacement of the ipcache import with
the nodemanager in the upstream main branch.

Signed-off-by: Maartje Eyskens <maartje.eyskens@isovalent.com>
@meyskens meyskens force-pushed the meyskens/backport-spire-status branch from f2f57ca to cfa4bc2 Compare September 4, 2023 15:35
@meyskens
Copy link
Member Author

meyskens commented Sep 4, 2023

/test-backport-1.14

@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 Sep 4, 2023
@youngnick youngnick merged commit b684802 into cilium:v1.14 Sep 5, 2023
56 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.14 This PR represents a backport for Cilium 1.14.x of a PR that was merged to main. kind/backports This PR provides functionality previously merged into master. ready-to-merge This PR has passed all tests and received consensus from code owners to merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants