GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — CLOUDSTACK-8725 RVR functionality is broken in case of isolated netwo… by bvbharatk · Pull Request #692 · apache/cloudstack

Conversation

@bvbharatk

@bvbharatk bvbharatk commented Aug 13, 2015

Copy link
Copy Markdown
Contributor

CLOUDSTACK-8725 RVR functionality is broken in case of isolated networks, conntrackd fails to start.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bvbharat
CLOUDSTACK-8725 RVR functionality is broken in case of isolated netwo…
fbebfd7 
…rks, conntrackd fails to start.
@asfbot

asfbot commented Aug 13, 2015

Copy link
Copy Markdown

cloudstack-pull-rats #290 SUCCESS
This pull request looks good
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
DaanHoogland
DaanHoogland reviewed Aug 13, 2015
View reviewed changes
Comment thread systemvm/patches/debian/config/opt/cloud/bin/cs/CsRedundant.py

@DaanHoogland DaanHoogland Aug 13, 2015

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

would it make sense to make sure it is marked as needed, or is this always needed (even in non redundant routers)?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@bvbharatk bvbharatk Aug 14, 2015

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Hi Daan,

This will be needed only in case of redundant router. The current code tries to copy this file only if it is a rvr. It will not be copied for normal routers.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@DaanHoogland DaanHoogland Aug 14, 2015

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

then wouldn't the keepalived.conf always be needed as well? the use of copy_if_needed seemed very intentional.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@bvbharatk bvbharatk Aug 18, 2015

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Hi Daan,

The intent of copy_if_needed is to prevent overwriting the config file if it is preset already. In case of conntrackd, the config file is already present and so it dose not copy the default cloudstack conntrackd template and as a result conntrackd fails to start. For this(copy_if_needed) to work we should make sure that this file is not present when this code runs for the first time. There may be similar errors related to rvr. I was trying to see if i can address them as a part of larger fix.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@asfbot

asfbot commented Aug 13, 2015

Copy link
Copy Markdown

cloudstack-pull-requests #987 ABORTED
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@asfbot

asfbot commented Aug 13, 2015

Copy link
Copy Markdown

cloudstack-pull-analysis #223 UNSTABLE
Looks like there's a problem with this pull request
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@remibergsma

remibergsma commented Aug 25, 2015

Copy link
Copy Markdown
Contributor

@bvbharatk Had a quick look and it seems to solve the problem. Can you please rebase against latest master so we can have a clean merge? Thanks!

@DaanHoogland Any more questions on this one?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@DaanHoogland

DaanHoogland commented Aug 26, 2015

Copy link
Copy Markdown
Contributor

@remibergsma no, answer is clear and though my question expresses a reservation on design level it should not stop this PR; => LGTM
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@remibergsma

remibergsma commented Aug 26, 2015

Copy link
Copy Markdown
Contributor

@DaanHoogland OK, thanks!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@michaelandersen

michaelandersen commented Aug 26, 2015

Copy link
Copy Markdown

verified that conntrackd starts on redundant vpc routers.
LGTM in that respect.

Design wise i feel we need to refactor this, naming of copy_if_needed is ambiguous.
Also we're running a lot of boilerplate code which can be replaced by more widely used and maintained python packages.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@remibergsma

remibergsma commented Aug 26, 2015

Copy link
Copy Markdown
Contributor

@bvbharatk The rebase is needed due to the 'PEP8' style fix that has been done on CsRedundant.py Let me know if you need help :-)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
karuturi pushed a commit to karuturi/cloudstack that referenced this pull request Aug 31, 2015
@bvbharat @karuturi
CLOUDSTACK-8725 RVR functionality is broken in case of isolated netwo…
18dbc0c 
…rks, conntrackd fails to start.

Rebased with master. This closes apache#692
@karuturi karuturi mentioned this pull request Aug 31, 2015
CLOUDSTACK-8725 RVR functionality is broken in case of isolated networks #763
Merged
@asfgit asfgit closed this in #763 Aug 31, 2015
asfgit pushed a commit that referenced this pull request Aug 31, 2015
@karuturi
Merge pull request #763 from karuturi/pr/692-rebase
1bbd23e 
CLOUDSTACK-8725 RVR functionality is broken in case of isolated networksconntrackd fails to start.

Rebased with master. This closes #692

* pr/763:
  CLOUDSTACK-8725 RVR functionality is broken in case of isolated networks, conntrackd fails to start.

Signed-off-by: Rajani Karuturi <[email protected]>
maneesha-p pushed a commit to maneesha-p/cloudstack that referenced this pull request Sep 3, 2015
@bvbharat Maneesha.P
CLOUDSTACK-8725 RVR functionality is broken in case of isolated netwo…
093ef63 
…rks, conntrackd fails to start.

Rebased with master. This closes apache#692
@remibergsma remibergsma mentioned this pull request Oct 2, 2015
CLOUDSTACK-8915 - Cannot SSH into VMs deployed Redundant VPC routers #908
Merged
yadvr pushed a commit that referenced this pull request Jan 20, 2021
@ravening Rakesh Venkatesh @yadvr
Fix typo in loading message (#692)
a32ba27 
Co-authored-by: Rakesh Venkatesh <[email protected]>
Signed-off-by: Rohit Yadav <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

6 participants

@bvbharatk @asfbot @remibergsma @DaanHoogland @michaelandersen @bvbharat
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.