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: Add service to detect clones on boot #3098

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

dheyay
Copy link
Contributor

@dheyay dheyay commented May 1, 2024

Fixes: #3068

Why is this needed?

This PR solves all of our problems because it updates the pro activity id on cloned attached containers

Test Steps

To test this:

  • Launch a container and attach to a pro subscription
  • Check the activity id
  • snapshot the container and clone it
lxc snapshot <container> <snapshot-name>
lxc copy <container>/<snapshot-name> <clone-name>
  • Check the activity ID on the clone container, it should be different

(also try multiple clones to check if the activity id changes)

Checklist

  • I have updated or added any unit tests accordingly
  • I have updated or added any integration tests accordingly
  • Changes here need to be documented, and this was done in:

Does this PR require extra reviews?

  • Yes
  • No

Copy link

github-actions bot commented May 1, 2024

Jira: This PR is not related to a Jira item. (The PR title does not include a SC-#### reference)

GitHub Issues:

Launchpad Bugs: No Launchpad bugs are fixed by this PR. (No commits have LP: #### references)

Documentation: The changes in this PR do not require documentation changes.

👍 this comment to confirm that this is correct.

@dheyay dheyay force-pushed the clone-detect-bug branch 2 times, most recently from 54a51ca to ae24c2e Compare May 2, 2024 01:14
@lucasmoura
Copy link
Contributor

@dheyay @orndorffgrant I wonder if we should instead reuse the ua-reboot-cmds for this. Even though this service requires a marker file to exist in order to run, I think we can create that marker file whenever we attach the machine. In that sense, a clone would also carry that marker file with it.

The downside of this approach is that we will now execute this service on every reboot of an attached machine. We could create another marker file to show that we should only run the metering part of the service, but this might be adding to much complexity to the service.

So let me know if we should further discuss this idea.

@orndorffgrant
Copy link
Collaborator

Upon reviewing I had yet another idea: we could store jobs-status.json in /run which would make just the normal timer (with OnStartupSec=1min) execute all jobs once on startup.
We can discuss more when we have time (in madrid or after)

@dheyay dheyay changed the base branch from next-v33 to main June 7, 2024 14:18
@dheyay dheyay marked this pull request as draft June 7, 2024 20:04
Copy link

github-actions bot commented Oct 7, 2024

PR Checklist

How to use this checklist

How to use this checklist

PR Author

For each section, check a box when it is true.
Uncheck a box if it becomes un-true.
Then check the box at the bottom of the PR description to re-run the action that creates this checklist.
The action that creates and updates this comment will retain your edits.
The action will fail if the checklist is not completed.

PR Reviewer

Check that the PR checklist action did not fail.
Double check that the author filled out the checklist accurately.
If you disagree with a checklist item, start a conversation.
For example, the author may say they don't think integration tests are necessary, but you may disagree.

Bug References

Confirm

  • I've properly referenced all bugs that this PR fixes
How to properly reference fixed bugs
  • If this PR is related to a Jira item, include an SC-1234 reference in the PR title
  • If this PR is fixes a GitHub issue, include a Fixes: #1234 reference in the commit that fixes the issue
  • If this PR is fixes a Launchpad bug, include a LP: #12345678 reference in the commit that fixes the issue

Test Updates

Unit Tests

  • I have updated or added any unit tests accordingly
  • No unit test changes are necessary for this change

Integration Tests

  • I have updated or added any integration tests accordingly
  • No integration test changes are necessary for this change

Documentation

  • Changes here need to be documented and I have referenced the docs PR in the description
  • No documentation updates are necessary for this change

Does this PR require review from someone outside the core ubuntu-pro-client team?

  • Yes, and I have requested those reviews via GitHub
  • No

@dheyay dheyay marked this pull request as ready for review October 7, 2024 21:41
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.

Bug: clones not noticed right away
3 participants