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

feat: remaining schema changes for indirect host audits #15787

Merged
merged 1 commit into from
Jan 29, 2025

Conversation

pb82
Copy link
Contributor

@pb82 pb82 commented Jan 29, 2025

SUMMARY

Remaining schema changes for indirect host audit:

New model & table IndirectManagedNodeAudit to store audit results
New column event_queries_processed to store whether a job has already been processed

ISSUE TYPE
  • New feature
COMPONENT NAME
  • API
AWX VERSION
devel
ADDITIONAL INFORMATION


name = CharField(max_length=255)

canonical_facts = JSONField(default=dict)
Copy link
Member

Choose a reason for hiding this comment

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

Should there be a uniqueness constraint on canonical_facts? @Ladas

Copy link
Contributor

@Ladas Ladas Jan 29, 2025

Choose a reason for hiding this comment

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

@AlanCoding hm I think 'unique_together': {('name', 'job')} is good, making managed node name, job_id combination unique.

(I wonder if "name" is enough/good given the table has ManagedNode in it, officially this is managed_node_name, but AWX already uses host_name in many places...)


What the processing code needs to do though, if we'll be getting these records from one job run:

name: "host1", job: "1", canonical_facts: {"serial_number: "XY"}
name: "host1", job: "1", canonical_facts: {"machine_uuid: "XX"}

it needs to merge them into(with the on-conflict clause of the upsert):

name: "host1", job: "1", canonical_facts: {"serial_number: "XY", "machine_uuid: "XX"}

now a case

name: "host1", job: "1", canonical_facts: {"serial_number: "XY", "machine_uuid: "ZZ"}
name: "host1", job: "1", canonical_facts: {"machine_uuid: "XX"}

we can solve with just storing the individual canonical facts/facts as array by default (with the on-conflict clause of the upsert):

name: "host1", job: "1", canonical_facts: {"serial_number: ["XY"], "machine_uuid: ["ZZ","XX"]}

------->
I think I'd prefer this approach, to approach with unique index on canonical facts...(a bit less data to store around) But that it's an alternative solution. We will be still doing deduplication for the reports, there the result is already something like:

name: "host1", canonical_facts: {names:['host1', 'host_2'],  "serial_number: ["XY"], "machine_uuid: ["ZZ","XX"]}

Copy link
Contributor

Choose a reason for hiding this comment

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

It would be good to cover these cases by tests, no matter which path we'll choose

@pb82 pb82 merged commit 2cc06a6 into indirect-host-counting Jan 29, 2025
9 of 20 checks passed
@pb82 pb82 deleted the AAP-37280 branch January 29, 2025 13:34
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.

3 participants