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

Refactor firewall reconcile. #71

Merged
merged 12 commits into from
Aug 18, 2020
Merged

Refactor firewall reconcile. #71

merged 12 commits into from
Aug 18, 2020

Conversation

Gerrit91
Copy link
Contributor

@Gerrit91 Gerrit91 commented Jun 15, 2020

  • Adapts to latest controller implementations of other extension providers (controller-library was not yet introduced, will be done for Gardener >=v1.5 controller)
  • Smaller functions for firewall reconcile
  • Compares firewall image properly and recreates firewall if image changed on shoot spec (respects semantic versioning)
  • Fixes a possible issue where a firewall could be deleted even if it does not belong to the cluster anymore

@Gerrit91 Gerrit91 force-pushed the refactor-firewall-reconcile branch from 117c902 to 19016c0 Compare June 15, 2020 10:34
@Gerrit91 Gerrit91 force-pushed the refactor-firewall-reconcile branch from 19016c0 to 87fd01c Compare June 15, 2020 10:37
@Gerrit91 Gerrit91 changed the title Reconcile firewall reconcile. Refavtor firewall reconcile. Jun 15, 2020
@Gerrit91 Gerrit91 changed the title Refavtor firewall reconcile. Refactor firewall reconcile. Jun 15, 2020
@Gerrit91 Gerrit91 force-pushed the refactor-firewall-reconcile branch from e36020f to 6e39cbe Compare June 15, 2020 14:30
@Gerrit91 Gerrit91 force-pushed the refactor-firewall-reconcile branch from 1e86d0c to 69c8518 Compare July 8, 2020 10:52
@Gerrit91
Copy link
Contributor Author

Gerrit91 commented Jul 8, 2020

I tried it out and it is working nicely. We can now do these kind of things as well:

$ c cluster update f960732b-9562-4ef8-b488-8d5fb8a99104 --firewallimage firewall-ubuntu-2.0

And it will result into:

...
gardener-extension-provider-metal-58fdf74dc4-wp2js gardener-extension-provider-metal {"level":"info","ts":"2020-07-08T11:20:36.879Z","logger":"infrastructure-actuator","msg":"firewall image has changed","clusterid":"f960732b-9562-4ef8-b488-8d5fb8a99104","machineid":"00000000-0000-0000-0000-ac1f6b7befb4","current":"firewall-ubuntu-2.0.20200617","new":"firewall-ubuntu-2.0.20200701"}
...

@Gerrit91 Gerrit91 requested a review from majst01 July 8, 2020 11:24
@Gerrit91 Gerrit91 requested a review from majst01 July 8, 2020 12:01
@@ -28,7 +28,7 @@ require (
github.com/grpc-ecosystem/go-grpc-middleware v1.1.0 // indirect
github.com/jetstack/cert-manager v0.6.2 // indirect
github.com/metal-stack/firewall-controller v0.1.0
github.com/metal-stack/metal-go v0.3.2
github.com/metal-stack/metal-go v0.7.8
Copy link
Contributor

Choose a reason for hiding this comment

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

Please update to the most recent version. Also here, we still refer to a ancient mcm:

github.com/gardener/machine-controller-manager => github.com/metal-pod/machine-controller-manager v0.0.0-20190801141331-4e2b75ebc6c0

This must be fixed as well

@majst01 majst01 merged commit 76c7d13 into master Aug 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants