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

Improve Advertisement Generation and Broadcasting #281

Merged
merged 1 commit into from Sep 23, 2020

Conversation

fraborg
Copy link
Member

@fraborg fraborg commented Sep 22, 2020

Description

In this PR

  • fix a bug in the remote-watcher, when we update the PeeringRequest with the status of the Advertisement.
  • improve the creation of images in the Advertisement filtering images with the same name

Minor changes

  • added Prefix constants to avoid typos errors in the name of resources

How Has This Been Tested?

  • Unit test to check image filtering
  • KinD test to check the PeeringRequest is correctly updated

used by the k8s cluster like the podCIDR and ClusterCIDR
description: this field is used by the IPAM embedded in the tunnelEndpointCreator
if the podCIDR of a peering cluster needs to be NATed a new subnet
from the 10.0.0.0/8 is used. if subnets belonging to that range
Copy link
Member

@palexster palexster Sep 22, 2020

Choose a reason for hiding this comment

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

The sentence starting with if is a bit hard to understand.
A possible proposal:
Reserved subnets listed in this field are excluded from the list of possible subnets used for natting POD CIDR. Add here the subnets already used in your environment.

Copy link
Member Author

Choose a reason for hiding this comment

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

@alacuku can you check if the new description of this field is ok? thanks!

@palexster palexster changed the title Improve Adv broadcaster Improve Advertisement Generation and Broadcasting Sep 22, 2020
@pull-request-size pull-request-size bot added size/L and removed size/M labels Sep 22, 2020
@coveralls
Copy link

coveralls commented Sep 22, 2020

Pull Request Test Coverage Report for Build 266973768

  • 16 of 19 (84.21%) changed or added relevant lines in 4 files are covered.
  • 8 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.03%) to 51.547%

Changes Missing Coverage Covered Lines Changed/Added Lines %
internal/advertisement-operator/broadcaster.go 11 14 78.57%
Files with Coverage Reduction New Missed Lines %
internal/peering-request-operator/peering-request-controller.go 3 73.68%
internal/peering-request-operator/foreign-cluster.go 5 78.43%
Totals Coverage Status
Change from base Build 266869904: -0.03%
Covered Lines: 6946
Relevant Lines: 13475

💛 - Coveralls

@palexster
Copy link
Member

palexster commented Sep 22, 2020

Ref. #281 Unlinking because this PR does not address this issue completely.

@palexster palexster added this to In progress in Bug Tracking Sep 22, 2020
@palexster palexster moved this from In progress to Review in Bug Tracking Sep 22, 2020
@palexster
Copy link
Member

/rebase

- use an intermediate map to avoid duplicate images
- added Prefix const to avoid hardcoded names for resources
@adamjensenbot
Copy link
Collaborator

Rebase status: success!

@AbakusW
Copy link
Member

AbakusW commented Sep 23, 2020

/rebase

@adamjensenbot
Copy link
Collaborator

Rebase status: success!

@palexster palexster merged commit 4429d3b into master Sep 23, 2020
Bug Tracking automation moved this from Review to Done Sep 23, 2020
@palexster palexster deleted the frb/broadcaster-upgrade branch September 23, 2020 07:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Broadcaster fails to update the PeeringRequest
5 participants