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

hubble-ca-cert ConfigMap cleanup #17294

Merged
merged 2 commits into from Nov 9, 2021
Merged

hubble-ca-cert ConfigMap cleanup #17294

merged 2 commits into from Nov 9, 2021

Conversation

kaworu
Copy link
Member

@kaworu kaworu commented Sep 2, 2021

This PR deletes the last references to the hubble-ca-cert ConfigMap that has been removed by #16900.

@kaworu kaworu added pending-review kind/cleanup This includes no functional changes. release-note/misc This PR makes changes that have no direct user impact. sig/hubble Impacts hubble server or relay area/helm Impacts helm charts and user deployment experience labels Sep 2, 2021
@kaworu kaworu self-assigned this Sep 2, 2021
@kaworu kaworu requested review from a team as code owners September 2, 2021 20:05
@kaworu kaworu requested review from a team September 2, 2021 20:05
jedsalazar
jedsalazar reviewed Sep 2, 2021
View reviewed changes
Copy link
Contributor

@jedsalazar jedsalazar left a comment

Choose a reason for hiding this comment

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

lgtm

gandro
gandro approved these changes Sep 14, 2021
View reviewed changes
test/helpers/kubectl.go
@@ -4378,7 +4378,7 @@ func (kub *Kubectl) CleanupCiliumComponents() {
wg sync.WaitGroup

resourcesToDelete = map[string]string{
"configmap": "cilium-config hubble-ca-cert hubble-relay-config",
Copy link
Member

@gandro gandro Sep 14, 2021 •
edited

Choose a reason for hiding this comment

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

Can we run a Jenkins CI test for this? I believe we can safely remove it, but since this function here is also used in the upgrade tests (which deploys Cilium v1.10, which afair still creates the hubble-ca-cert ConfigMap), I think it's worth running to ensure we don't break CI via leftover artifacts.

@kaworu
Copy link
Member Author

kaworu commented Sep 14, 2021

test-1.19-5.4

@kaworu
Copy link
Member Author

kaworu commented Sep 14, 2021

test-me-please

@kaworu kaworu force-pushed the pr/kaworu/hubble-ca-cert-configmap-cleanup branch from 046a39c to 73891a5 Compare September 23, 2021 11:19
@kaworu
Copy link
Member Author

kaworu commented Sep 23, 2021 •
edited by maintainer-s-little-helper bot

test-me-please

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

Click to show.

Test Name

K8sServicesTest Checks service across nodes Checks ClusterIP Connectivity

Failure Output

FAIL: Expected

If it is a flake, comment /mlh new-flake Cilium-PR-K8s-GKE 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

K8sIstioTest Istio Bookinfo Demo Tests bookinfo inter-service connectivity

Failure Output

FAIL: unable to deploy Istio

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.

@kaworu kaworu force-pushed the pr/kaworu/hubble-ca-cert-configmap-cleanup branch from 73891a5 to ec72789 Compare September 27, 2021 15:17
@kaworu kaworu force-pushed the pr/kaworu/hubble-ca-cert-configmap-cleanup branch from ec72789 to cac583f Compare October 18, 2021 08:38
@kaworu
Copy link
Member Author

kaworu commented Oct 18, 2021 •
edited by maintainer-s-little-helper bot

/test

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

Click to show.

Test Name

K8sCLI CLI Identity CLI testing Test cilium bpf metrics list

Failure Output

FAIL: testapp pods are not ready after timeout in namespace "202110181127k8sclicliidentityclitestingtestciliumbpfmetricslist"

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

@pchaigno
Copy link
Member

pchaigno commented Nov 8, 2021

@kaworu I think you'll need to rebase and retrigger the end-to-end tests as they have changed since last time (updated k8s version).

@kaworu kaworu force-pushed the pr/kaworu/hubble-ca-cert-configmap-cleanup branch from cac583f to b0d1b54 Compare November 8, 2021 09:11
@kaworu
helm: remove hubble-ca-cert from certgen authorization
9020a77 
For context, our Helm charts have removed the generation of the
hubble-ca-cert ConfigMap through certgen since
21fa834.

This patch remove the RBAC authorization allowing to update this
ConfigMap from the ClusterRole used by certgen.

Signed-off-by: Alexandre Perrin <alex@kaworu.ch>
@kaworu
test: remove unused configmap from CleanupCiliumComponents()
b0d1b54 
The hubble-ca-cert ConfigMap is not installed by Cilium anymore, see
cilium#16900.

This patch remove this ConfigMap from the CleanupCiliumComponents() list
of resources to delete.

Signed-off-by: Alexandre Perrin <alex@kaworu.ch>
@kaworu
Copy link
Member Author

kaworu commented Nov 8, 2021

/test

@pchaigno
Copy link
Member

pchaigno commented Nov 9, 2021 •
edited

Provisioning issue was just fixed:
/test

@pchaigno
Copy link
Member

pchaigno commented Nov 9, 2021

k8s-1.16-kernel-netnext failed with known flake #16813. Reviews are covered. Merging.

@pchaigno pchaigno added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Nov 9, 2021
@pchaigno pchaigno merged commit 9a1086e into cilium:master Nov 9, 2021
@kaworu kaworu deleted the pr/kaworu/hubble-ca-cert-configmap-cleanup branch November 11, 2021 08:11
@kaworu kaworu mentioned this pull request Dec 2, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jedsalazar jedsalazar jedsalazar left review comments

@gandro gandro gandro approved these changes

@nathanjsweet nathanjsweet nathanjsweet approved these changes

@rolinh rolinh rolinh approved these changes

@nbusseneau nbusseneau nbusseneau approved these changes

@michi-covalent michi-covalent michi-covalent approved these changes

@seanmwinn seanmwinn Awaiting requested review from seanmwinn

Assignees

@kaworu kaworu

@seanmwinn seanmwinn

Labels
area/helm Impacts helm charts and user deployment experience kind/cleanup This includes no functional changes. 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. sig/hubble Impacts hubble server or relay
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
9 participants
@kaworu @pchaigno @gandro @nathanjsweet @rolinh @nbusseneau @jedsalazar @michi-covalent @seanmwinn