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

Filter out icmpv6 when reading back ec2 security groups. #16383

Merged
merged 1 commit into from Oct 9, 2023

Conversation

hpidcock
Copy link
Member

@hpidcock hpidcock commented Oct 5, 2023

Firewaller is crashing when trying to read rules out of the ec2 provider.

machine-0: 13:26:54 ERROR juju.worker.dependency "firewaller" manifold worker returned unexpected error: invalid destination for ingress rule: invalid protocol "icmpv6", expected "tcp", "udp", or "icmp"

This is due to #16061 adding in the minimum icmpv6 rules for proper v6 support in aws that should, until juju models this better, be hidden from the firewaller.

QA steps

@tlm how best to test this?

Documentation changes

N/A

Links

https://jenkins.juju.canonical.com/job/test-deploy-test-deploy-bundles-aws/3336/

@hpidcock hpidcock requested a review from tlm October 5, 2023 10:48
@hpidcock hpidcock added the 3.1 label Oct 5, 2023
@tlm
Copy link
Member

tlm commented Oct 6, 2023

Best way to test this is to bootstrap with the following:

juju bootstrap --bootstrap-constraints "arch=amd64" --config vpc-id=vpc-042e10f8faf56df93 aws/ap-southeast-2

You can then confirm from the logs that we are not seeing the firewater crash from reading in the icmpv6 rules.

I have tested this on my end and can't see any issues.

Copy link
Member

@tlm tlm left a comment

Choose a reason for hiding this comment

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

lgtm

@tlm
Copy link
Member

tlm commented Oct 6, 2023

We should loop back around soon and give Juju icmpv6 support. That would be the best outcome.

@hpidcock
Copy link
Member Author

hpidcock commented Oct 9, 2023

/merge

@jujubot jujubot merged commit 083f92e into juju:3.1 Oct 9, 2023
20 of 21 checks passed
@jack-w-shaw jack-w-shaw mentioned this pull request Oct 9, 2023
jujubot added a commit that referenced this pull request Oct 9, 2023
#16399

Merges:
- #16321
- #16361
- #16373
- #16387
- #16386
- #16389
- #16391
- #16398
- #16383
- #16366

Conflicts:
- upgrades/operations.go
- upgrades/upgrade_test.go

All conflicts from #16366. Arising simply from the version number in upgrade_steps

I also needed to rename steps_317.go to steps_324.go
@jack-w-shaw jack-w-shaw mentioned this pull request Oct 9, 2023
jujubot added a commit that referenced this pull request Oct 9, 2023
#16401

Merges:
- #16354
- #16321
- #16361
- #16373
- #16377
- #16387
- #16386
- #16389
- #16392
- #16391
- #16398
- #16383
- #16366
- #16399

Conflicts:
- state/upgrades.go
- state/upgrades_test.go
- upgrades/backend.go
- upgrades/operations.go
- upgrades/upgrade_test.go

All conflicts resulting from #16366 

Since this is an unreleased minor version, we do not need upgrade steps (there is nowhere to upgrade from), so drop them
@ycliuhw ycliuhw mentioned this pull request Oct 10, 2023
jujubot added a commit that referenced this pull request Oct 11, 2023
#16407

Merge `3.3` into `main`:
- #16354
- #16321
- #16361
- #16373
- #16377
- #16387
- #16389
- #16392
- #16391
- #16398
- #16383
- #16366
- #16367
- #16382

```
# Conflicts:
# apiserver/facades/client/application/application_unit_test.go
# apiserver/facades/client/secrets/secrets.go
# apiserver/facades/controller/caasoperatorprovisioner/provisioner_test.go
# apiserver/facades/controller/caasunitprovisioner/provisioner_test.go
# caas/kubernetes/provider/application/application_test.go
# caas/kubernetes/provider/k8s_test.go
# caas/kubernetes/provider/operator_test.go
# internal/docker/registry/internal/gitlab.go
# state/secrets_test.go
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants