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.

Sign up for GitHub

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

Jump to bottom

Adding Uptime tracking capability #1091

Merged
merged 2 commits into from
Apr 26, 2024
Merged

Adding Uptime tracking capability #1091

merged 2 commits into from
Apr 26, 2024

Conversation

chajain
Copy link
Collaborator

@chajain chajain commented Feb 13, 2024 •
edited
Loading

Description

Uptime data is included in the data sent to SCC via RMT
https://jira.suse.com/browse/CSE-504

Fixes # (issue)

Fixes # (issue)

Change Type

Please select the correct option.

  • Bug Fix (a non-breaking change which fixes an issue)
  • New Feature (a non-breaking change which adds new functionality)
  • Documentation Update (a change which only updates documentation)

Checklist

Please check off each item if the requirement is met.

  • I have verified that my code follows RMT's coding standards with rubocop.
  • I have reviewed my own code and believe that it's ready for an external review.
  • I have provided comments for any hard-to-understand code.
  • I have documented the MANUAL.md file with any changes to the user experience.
  • RMT's test coverage remains at 100%.
  • If my changes are non-trivial, I have added a changelog entry to notify users at package/obs/rmt-server.changes.

Other Notes

Please use this space to provide notes or thoughts to the team, such as tips on how to review/demo your changes.
How to test this PR

On the client system clone SUSE/connect-ng#202 ( this PR has the connect-ng changes for uptime tracking feature )
Also clone https://github.com/SUSE/uptime-tracker
On the RMT system, clone this code
Execute rails db:migrate to create the system_uptimes table
Start the server bin/rails server -b 0.0.0.0
Register the system on client system using command suseconnect --url <rmt_system_ip> , it should populate the system_uptimes table with the entries from /etc/zypp/suse-uptime.logs on client system

@chajain chajain changed the title WIP: Adding Uptime tracking capability Adding Uptime tracking capability Feb 28, 2024
@chajain chajain force-pushed the uptime_tracking branch 2 times, most recently from 46ad8c1 to e98f21b Compare March 6, 2024 01:35
@chajain chajain requested a review from felixsch March 6, 2024 15:26
ngetahun
ngetahun requested changes Mar 13, 2024
View reviewed changes
db/schema.rb Outdated Show resolved Hide resolved
lib/suse/connect/system_serializer.rb Outdated Show resolved Hide resolved
package/files/systemd/rmt-uptime-cleanup.timer Outdated Show resolved Hide resolved
db/migrate/20240111200053_create_system_uptimes.rb Outdated Show resolved Hide resolved
@yarunachalam yarunachalam force-pushed the uptime_tracking branch 2 times, most recently from 02b9f06 to a1d4638 Compare March 29, 2024 17:25
@yarunachalam yarunachalam requested a review from ngetahun March 29, 2024 17:26
@rjschwei
Copy link
Member

rjschwei commented Apr 1, 2024

AFIK version bumps are handled independently, i.e. lib/rmt.rb and the spec file changes should not be part of the PR. The Public Cloud team expects support for Debian repositories as the primary feature of 2.16, not sure is that is merged yet.

@digitaltom
Copy link
Member

digitaltom commented Apr 2, 2024 •
edited
Loading

AFIK version bumps are handled independently, i.e. lib/rmt.rb and the spec file changes should not be part of the PR.
The Public Cloud team expects support for Debian repositories as the primary feature of 2.16, not sure is that is merged yet.

Yes, please just add a note about the changes to the changelog, we'll include it into the next release and take care about the versioning.

ngetahun
ngetahun approved these changes Apr 11, 2024
View reviewed changes
Copy link
Contributor

@ngetahun ngetahun left a comment

Choose a reason for hiding this comment

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

LGTM!

@yarunachalam
Copy link
Collaborator

AFIK version bumps are handled independently, i.e. lib/rmt.rb and the spec file changes should not be part of the PR.
The Public Cloud team expects support for Debian repositories as the primary feature of 2.16, not sure is that is merged yet.

Yes, please just add a note about the changes to the changelog, we'll include it into the next release and take care about the versioning.

Do I need to remove the spec file from this PR ?

@digitaltom
Copy link
Member

Do I need to remove the spec file from this PR ?

I think you have changes in the .spec file that you need to keep regarding the timers. But you don't need to change the version number. Just add a description of the change to the changelog, and the team will take care about the versioning + release.

@yarunachalam yarunachalam force-pushed the uptime_tracking branch 3 times, most recently from b905549 to ca7bbec Compare April 15, 2024 21:17
@yarunachalam yarunachalam force-pushed the uptime_tracking branch 2 times, most recently from 1554d5f to a6f7cbe Compare April 15, 2024 22:31
@yarunachalam yarunachalam requested a review from digitaltom April 17, 2024 14:40
@yarunachalam yarunachalam force-pushed the uptime_tracking branch 3 times, most recently from ee06af4 to f1640eb Compare April 18, 2024 18:04
@yarunachalam yarunachalam requested a review from digitaltom April 19, 2024 17:41
Adnilson
Adnilson requested changes Apr 22, 2024
View reviewed changes
Copy link
Contributor

@Adnilson Adnilson left a comment •
edited
Loading

Choose a reason for hiding this comment

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

We can correct some variable inconsistency.

There is system_uptime and sometimes systemuptime.

Can we have this changed? It also helps with reading.

EDIT: I have an explanation related here: https://github.com/SUSE/happy-customer/pull/7339#pullrequestreview-2014262074

@yarunachalam yarunachalam force-pushed the uptime_tracking branch 2 times, most recently from 6822772 to 4fdc90b Compare April 24, 2024 16:41
@yarunachalam yarunachalam requested a review from Adnilson April 24, 2024 18:44
@chajain chajain mentioned this pull request Apr 25, 2024
Merged
@chajain
Copy link
Collaborator Author

chajain commented Apr 25, 2024

Performed the end to end testing along with SCC latest PR https://github.com/SUSE/happy-customer/pull/7339 and connect-ng PR SUSE/connect-ng#210 and verified following scenarios

  • Creation of system_uptime table with db:migrate
  • Populating the system_uptime table when the keepalive heartbeat is sent from client
  • rmt-cli systems scc-sync syncs the system_uptime tables on SCC from RMT

Adnilson
Adnilson reviewed Apr 26, 2024
View reviewed changes
spec/lib/suse/connect/system_serializer_spec.rb
@@ -86,4 +86,13 @@
expect(serializer.key? :system_token).to eq(false)
end
end

context 'system with systemuptime' do
Copy link
Contributor

Choose a reason for hiding this comment

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

I would also change the description but I don't see it as a blocker.

@digitaltom digitaltom merged commit d28e611 into master Apr 26, 2024
3 checks passed
@digitaltom digitaltom deleted the uptime_tracking branch April 26, 2024 08:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ngetahun ngetahun ngetahun approved these changes

@Adnilson Adnilson Adnilson approved these changes

@felixsch felixsch Awaiting requested review from felixsch

@yarunachalam yarunachalam Awaiting requested review from yarunachalam

@digitaltom digitaltom Awaiting requested review from digitaltom

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@chajain @rjschwei @digitaltom @yarunachalam @Adnilson @ngetahun