Skip to content

VR: do not restart conntrackd if no guest networks #7265

Merged
yadvr merged 1 commit intoapache:4.18from
weizhouapache:4.18-fix-conntrackd-start-in-vr
May 11, 2023
Merged

VR: do not restart conntrackd if no guest networks #7265
yadvr merged 1 commit intoapache:4.18from
weizhouapache:4.18-fix-conntrackd-start-in-vr

Conversation

@weizhouapache
Copy link
Copy Markdown
Member

Description

This PR fixes #6702

The service conntrackd will be started by the script /opt/cloud/bin/cs/CsRedundant.py, therefore add it to disabled_svcs so it is not started automatically when VR is started.

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)

Feature/Enhancement Scale or Bug Severity

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

@sonarqubecloud
Copy link
Copy Markdown

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@codecov
Copy link
Copy Markdown

codecov bot commented Feb 20, 2023

Codecov Report

Merging #7265 (2fd6cfb) into main (d12e9cb) will increase coverage by 0.00%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##               main    #7265   +/-   ##
=========================================
  Coverage     12.67%   12.67%           
- Complexity     8639     8641    +2     
=========================================
  Files          2716     2716           
  Lines        256112   256112           
  Branches      39926    39926           
=========================================
+ Hits          32456    32461    +5     
+ Misses       219528   219522    -6     
- Partials       4128     4129    +1     
Impacted Files Coverage Δ
...com/cloud/agent/manager/ConnectedAgentAttache.java 37.50% <0.00%> (+12.50%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@weizhouapache
Copy link
Copy Markdown
Member Author

@blueorangutan package

@blueorangutan
Copy link
Copy Markdown

@weizhouapache a Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan
Copy link
Copy Markdown

Packaging result: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 5614

@weizhouapache
Copy link
Copy Markdown
Member Author

@blueorangutan test matrix

@blueorangutan
Copy link
Copy Markdown

@weizhouapache a Trillian-Jenkins matrix job (centos7 mgmt + xenserver71, rocky8 mgmt + vmware67u3, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests

@blueorangutan
Copy link
Copy Markdown

Trillian test result (tid-6206)
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server r8
Total time taken: 45420 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7265-t6206-vmware-67u3.zip
Smoke tests completed. 108 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File

@blueorangutan
Copy link
Copy Markdown

Trillian test result (tid-6207)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 49935 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7265-t6207-kvm-centos7.zip
Smoke tests completed. 107 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File
test_08_upgrade_kubernetes_ha_cluster Error 4032.16 test_kubernetes_clusters.py
test_09_delete_kubernetes_ha_cluster Failure 0.05 test_kubernetes_clusters.py
ContextSuite context=TestKubernetesCluster>:teardown Error 79.84 test_kubernetes_clusters.py

@blueorangutan
Copy link
Copy Markdown

Trillian test result (tid-6205)
Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
Total time taken: 50188 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7265-t6205-xenserver-71.zip
Smoke tests completed. 107 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File
test_08_upgrade_kubernetes_ha_cluster Error 4067.60 test_kubernetes_clusters.py
test_09_delete_kubernetes_ha_cluster Failure 0.09 test_kubernetes_clusters.py
ContextSuite context=TestKubernetesCluster>:teardown Error 85.80 test_kubernetes_clusters.py

@kiranchavala
Copy link
Copy Markdown
Member

Verified the issue LGTM

Found that conntrackd service is disabled when using vpc redundant offering

Please find the screenshots

Before fix

conntracktd

After fix

conntracktd-pr

@weizhouapache
Copy link
Copy Markdown
Member Author

weizhouapache commented Mar 1, 2023

@kiranchavala
thanks for testing. it looks like expected behaviour

@DaanHoogland DaanHoogland added this to the 4.18.1.0 milestone Mar 1, 2023
@yadvr yadvr requested review from DaanHoogland and shwstppr May 8, 2023 07:14
if restart_conntrackd:
CsHelper.service("conntrackd", "restart")
else:
CsHelper.service("conntrackd", "stop")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

isn´t ignoring the contracd service here enough?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@DaanHoogland
it is better to stop it

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ok

Copy link
Copy Markdown
Contributor

@shwstppr shwstppr left a comment

Choose a reason for hiding this comment

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

LGTM. Test by @kiranchavala 👍

@weizhouapache weizhouapache changed the base branch from main to 4.18 May 8, 2023 11:01
@weizhouapache
Copy link
Copy Markdown
Member Author

@blueorangutan package

@blueorangutan
Copy link
Copy Markdown

@weizhouapache a Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan
Copy link
Copy Markdown

Packaging result: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 6043

@weizhouapache
Copy link
Copy Markdown
Member Author

@blueorangutan test

Copy link
Copy Markdown
Member

@yadvr yadvr left a comment

Choose a reason for hiding this comment

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

LGTM, manual testing and previous smoketests LGTM. Re-testing isn't required as changes are striclty in the VR codebase and has been manually tested by @kiranchavala

@apache apache deleted a comment from blueorangutan May 11, 2023
@apache apache deleted a comment from blueorangutan May 11, 2023
@yadvr yadvr merged commit 8c546bd into apache:4.18 May 11, 2023
@weizhouapache weizhouapache deleted the 4.18-fix-conntrackd-start-in-vr branch June 23, 2023 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Router displaying error "Failed to start Conntrack Deamon"

7 participants