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

Move lfmerge container to separate pod using pod affinity #1407

Merged
merged 3 commits into from
May 24, 2022

Conversation

rmunn
Copy link
Collaborator

@rmunn rmunn commented May 23, 2022

Description

As a follow-up to #1389, this PR moves the lfmerge container into a separate pod, using pod affinity to ensure that it will be assigned to the same node as the main app. (Since they share a persistent volume, this is necessary so that the volume can be mounted in ReadWriteOnce mode instead of ReadWriteMany — ReadWriteMany greatly limits the number of storage backends available, while ReadWriteOnce is a lot easier for storage backends to use. (I see https://longhorn.io/blog/longhorn-v1.1.0/ from January 2021 that announces ReadWriteMany availability in Longhorn, so perhaps the pod affinity can be gotten rid of at some point).

Fixes #1406.

Type of Change

Only keep lines below that describe this change, then delete the rest.

  • New feature (non-breaking change which adds functionality)

Screenshots

Please provide screenshots / animations for any change that involves the UI. Please provide animations to demonstrate user interaction / behavior changes

Testing on your branch

Please describe how to test and/or verify your changes. Provide instructions so we can reproduce. Please also provide relevant test data as necessary. These instructions will be used for QA testing below.

  • Ensure LF app still builds with make
  • Clone a project with Send/Receive and make sure LfMerge still runs

This PR makes basically no changes to the docker-compose build (one very minor change), so I don't expect it to have any effect. Most of the testing will need to be on the QA server.

Checklist

  • I have performed a self-review of my own code
  • I have reviewed the title/description of this PR which will be used as the squashed PR commit message
  • I have commented my code, particularly in hard-to-understand areas
  • I have added tests that prove my fix is effective or that my feature works

qa.languageforge.org testing

Reviewers: add/replace your name below and check the box to sign-off/attest the feature works as expected on qa.languageforge.org

  • Reviewer1 (YYYY-MM-DD HH:MM)
  • Reviewer2 (YYYY-MM-DD HH:MM)

rmunn added 3 commits May 21, 2022 09:42
Two env values in the lfmerge container were being read as an int and a
boolean, when Kubernetes requires them all to be strings. So we quote
them to ensure they will be strings.
Move lfmerge container to separate pod, using pod affinity to force it
to always run on the same node as the app container so that they can
share the sendreceive volume.
The lfmerge container doesn't need any files from the languageforge repo
at all, so let's make its Docker build context as small as possible.
That way it's a nearly-instant build once the image has downloaded.
@github-actions
Copy link

Unit Test Results

    1 files      1 suites   8s ⏱️
373 tests 373 ✔️ 0 💤 0

Results for commit 3284b0f.

Copy link
Contributor

@longrunningprocess longrunningprocess left a comment

Choose a reason for hiding this comment

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

well, I like the way this looks a lot more, let's hope it works! 🤞🏻

@rmunn rmunn merged commit 21c18c8 into develop May 24, 2022
@rmunn rmunn deleted the chore/lfmerge-separate-pod branch May 24, 2022 02:57
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.

feat: Move LfMerge container to separate pod
2 participants