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

Bug fixing and enhancement network operators #268

Merged
merged 1 commit into from
Sep 18, 2020
Merged

Conversation

alacuku
Copy link
Member

@alacuku alacuku commented Sep 16, 2020

Description

This PR brings bugfixing and some refactoring of the network operators

TunnelEndpointCreator

The tunnelendpoints.net.liqo.io resource is created now in one step, specifying the spec and status field at once. It used to first create the resource with the spec field, wait for the creation and then update the status field. This process is now delegated to the client-go library.
A newly created tunnelendpoint resource has the status field named phase set to Ready. This is needed for the tunnel and route operators. Otherwise the resource may be consumed before that some status fields are set.

Route-Operator

  • Correcting some misleading information in log messages.
  • Fixing a bug causing the LIQONET-POSTROUTING chain not to be flushed when the operator pod is removed
  • The reconcile function is idempotent with a resync period of 30 seconds where it checks that routing table and iptables rules are correctly configured.
  • When iptables rules and routes are added or removed events are sent to the resource been reconciled
    tep_events
    This way if an error occurs the user knows which instance/node to check for misconfiguration. If an error occurs while removing the external resources it is reported through an event on the related tunnelendpoint resource and waits for the user to solve the conflicts.

Tunnel-Operator

  • Fixing the bug which caused the operator not to reconcile the tunnelendpoints resources if restarted.
  • Periodically ensures the existence of the tunnel network interface for the peering clusters
  • The creation of the tunnel network interface is now idempotent: if it exists a check if the configuration is the same is done and it is recreated only when the existing one and the wanted one differs.
  • Instal/uninstall of network interfaces is reported by means of events to the related tunnelendpoint resource.

Fixes #(issue)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Please also note any relevant details for your test configuration.

Existing unit and integration test have been updated to use the status field phase with new value which for now could be set to Ready only.

@coveralls
Copy link

coveralls commented Sep 17, 2020

Pull Request Test Coverage Report for Build 260901563

  • 51 of 119 (42.86%) changed or added relevant lines in 4 files are covered.
  • 29 unchanged lines in 8 files lost coverage.
  • Overall coverage increased (+0.08%) to 50.805%

Changes Missing Coverage Covered Lines Changed/Added Lines %
internal/liqonet/route-operator.go 16 21 76.19%
internal/liqonet/tunnelEndpointCreator-operator.go 35 40 87.5%
pkg/liqonet/gretun.go 0 25 0.0%
internal/liqonet/tunnel-operator.go 0 33 0.0%
Files with Coverage Reduction New Missed Lines %
pkg/liqonet/route.go 2 4.4%
internal/liqonet/tunnelEndpointCreator-operator.go 3 65.33%
internal/liqonet/tunnel-operator.go 3 0%
internal/peering-request-operator/peering-request-controller.go 3 69.39%
pkg/liqonet/route_mock.go 3 80.7%
internal/liqonet/route-operator.go 4 73.84%
internal/peering-request-operator/foreign-cluster.go 5 78.43%
pkg/liqonet/utils.go 6 20.13%
Totals Coverage Status
Change from base Build 260900016: 0.08%
Covered Lines: 6752
Relevant Lines: 13290

💛 - Coveralls

@alacuku alacuku force-pushed the kcl/networkmodule branch 6 times, most recently from 94d501b to 62e91c6 Compare September 17, 2020 15:24
@alacuku alacuku changed the title [WIP]Removing the "phase" field from tunnelendpoints.net.liqo.io api [WIP]Bug fixing and enhancement network operators Sep 17, 2020
@alacuku alacuku changed the title [WIP]Bug fixing and enhancement network operators Bug fixing and enhancement network operators Sep 17, 2020
@@ -196,7 +219,7 @@ func (r *RouteController) createAndInsertIPTablesChains() error {
} else {
log.Info("created", "chain", LiqonetPreroutingChain, "in table", NatTable)
}
r.IPTablesChains[LiqonetPostroutingChain] = liqonetOperator.IPTableChain{
r.IPTablesChains[LiqonetPreroutingChain] = liqonetOperator.IPTableChain{
Copy link
Member

Choose a reason for hiding this comment

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

Out of curiosity: Why do we moved to PreRoutingChain?

Copy link
Member Author

Choose a reason for hiding this comment

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

it was a little bug where we saved the LiqonetPreroutingChain as the LiqonetPostroutingChain in the IPTablesChains table. Looking at line 217 it is creating the LiqonetPreroutingChain.

@alacuku alacuku force-pushed the kcl/networkmodule branch 2 times, most recently from 412f101 to 6ce0f3f Compare September 17, 2020 18:25
@alacuku
Copy link
Member Author

alacuku commented Sep 18, 2020

/rebase

@adamjensenbot
Copy link
Collaborator

Rebase status: success!

internal/liqonet/route-operator.go Outdated Show resolved Hide resolved
internal/liqonet/route-operator_test.go Outdated Show resolved Hide resolved
@palexster
Copy link
Member

/rebase

…is set "Ready" when it can be processed by the route and tunnnel operators

Bugfixing network operators
@adamjensenbot
Copy link
Collaborator

Rebase status: success!

@palexster palexster merged commit 9acb4a1 into master Sep 18, 2020
@palexster palexster deleted the kcl/networkmodule branch September 18, 2020 10:54
@alacuku alacuku mentioned this pull request Sep 23, 2020
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants