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.

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

daemons: name init functions and have one init #17616

Merged
merged 1 commit into from Oct 22, 2021
Merged

daemons: name init functions and have one init #17616

merged 1 commit into from Oct 22, 2021

Conversation

nebril
Copy link
Member

@nebril nebril commented Oct 15, 2021

This change refactors and names 3 of cmd/daemon init functions so
that the order of their execution and what they do is clearer.

@nebril nebril requested a review from a team October 15, 2021 11:48
@nebril nebril requested a review from a team as a code owner October 15, 2021 11:48
@nebril nebril requested a review from ldelossa October 15, 2021 11:48
@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 Oct 15, 2021
@nebril nebril added the release-note/minor This PR changes functionality that users may find relevant to operating Cilium. label Oct 15, 2021
@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 Oct 15, 2021
@nebril nebril added dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. release-note/misc This PR makes changes that have no direct user impact. and removed release-note/minor This PR changes functionality that users may find relevant to operating Cilium. labels Oct 15, 2021
@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 Oct 15, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.10.6 Oct 15, 2021
@nebril
Copy link
Member Author

nebril commented Oct 15, 2021 •
edited by maintainer-s-little-helper bot

/test

Job 'Cilium-PR-K8s-GKE' hit: #17204 (91.32% similarity)

@maintainer-s-little-helper maintainer-s-little-helper bot mentioned this pull request Oct 15, 2021
Closed
joestringer
joestringer approved these changes Oct 15, 2021
View reviewed changes
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.

Thanks, I've been thinking about doing something similar for a while. Operator could also do with some similar treatment, but this is good for now 👌

@nebril nebril requested a review from a team as a code owner October 18, 2021 10:47
@nebril nebril requested a review from a team October 18, 2021 10:47
@nebril nebril removed the request for review from a team October 20, 2021 09:42
@nebril
daemons: name init functions and have one init
19e2a59 
This change refactors and names 3 of `cmd/daemon` `init` functions so
that the order of their execution and what they do is clearer.

Signed-off-by: Maciej Kwiek <maciej@isovalent.com>
@nebril
Copy link
Member Author

nebril commented Oct 20, 2021

/test

@nebril
Copy link
Member Author

nebril commented Oct 20, 2021 •
edited by maintainer-s-little-helper bot

/test

Job 'Cilium-PR-K8s-1.21-kernel-4.9' failed and has not been observed before, so may be related to your PR:

Click to show.

Test Name

K8sDatapathConfig Host firewall With VXLAN

Failure Output

FAIL: Found 1 io.cilium/app=operator logs matching list of errors that must be investigated:

If it is a flake, comment /mlh new-flake Cilium-PR-K8s-1.21-kernel-4.9 so I can create a new GitHub issue to track it.

Job 'Cilium-PR-K8s-1.16-net-next' failed and has not been observed before, so may be related to your PR:

Click to show.

Test Name

K8sDatapathConfig Encapsulation Check vxlan connectivity with per-endpoint routes

Failure Output

If it is a flake, comment /mlh new-flake Cilium-PR-K8s-1.16-net-next so I can create a new GitHub issue to track it.

@nebril
Copy link
Member Author

nebril commented Oct 21, 2021 •
edited by maintainer-s-little-helper bot

/mlh new-flake Cilium-PR-K8s-1.21-kernel-4.9

👍 created #17660

@nebril
Copy link
Member Author

nebril commented Oct 21, 2021 •
edited by maintainer-s-little-helper bot

/mlh new-flake Cilium-PR-K8s-1.16-net-next

👍 created #17661

@twpayne
Copy link
Contributor

twpayne commented Oct 22, 2021

The changes are clear and the failures are unrelated to the changes, so marking as ready-to-merge.

@twpayne twpayne added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Oct 22, 2021
@nebril nebril merged commit f5a1621 into cilium:master Oct 22, 2021
@nebril nebril deleted the pr/nebril/consolidate-init branch October 22, 2021 11:26
@nebril nebril mentioned this pull request Oct 28, 2021
Merged
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.10 in 1.10.6 Oct 28, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.10 to Backport done to v1.10 in 1.10.6 Nov 26, 2021
@joestringer joestringer mentioned this pull request Dec 10, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joestringer joestringer joestringer approved these changes

@twpayne twpayne twpayne approved these changes

@ldelossa ldelossa ldelossa approved these changes

@errordeveloper errordeveloper Awaiting requested review from errordeveloper errordeveloper was automatically assigned from cilium/metrics

Assignees

@errordeveloper errordeveloper

Labels
ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
No open projects
1.10.6
Backport done to v1.10
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@nebril @twpayne @joestringer @ldelossa @errordeveloper @qmonnet