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

[COST-3814] Add Azure processing to subs flow #4754

Closed
wants to merge 24 commits into from

Conversation

cgoodfred
Copy link
Contributor

@cgoodfred cgoodfred commented Oct 28, 2023

Jira Ticket

COST-3814

Description

This change will add Azure processing to the subs flow. It extends the AWS processing to handle Azure sources. We are processing the same information as before with an additional tag option of com_redhat_rhel_instance to allow someone to add that tag instead of giving the additional Reader permission to the Azure scope.

Example of a kafka message that would be sent:

b'{"event_id": "f2105b64-1f1c-4fd6-bbaa-3ca6411f5942", "event_source": "cost-management", "event_type": "snapshot", "account_number": "10001", "org_id": "1234567", "service_type": "RHEL System", "instance_id": "<my-instance-id>", "timestamp": "2023-12-10 23:00:00+00:00", "expiration": "2023-12-11 00:00:00+00:00", "measurements": [{"value": "1", "uom": "vCPUs"}], "cloud_provider": "Azure", "hardware_type": "Cloud", "product_ids": ["69", "204"], "role": "Red Hat Enterprise Linux Server", "sla": "Premium", "usage": "Production", "billing_provider": "azure", "billing_account_id": "<my-billing-id>"}'

Testing

  1. Checkout Branch
  2. Restart Koku
  3. Add an Azure source with the required com_redhat_rhel tag on a VM. (reach out to Corey if you need credentials for a source that has this)
  4. Watch as we process azure subs records and query Azure for the VMID and then attempt to send a message to the subscriptions folks.

Notes

The AWS smokes label was run to ensure no regression in the AWS Subs Processing flow since there are not yet smokes for the Azure flow.

Example from setting only AWS enabled in the environment to give us more control over where different providers process:

subs_worker  | [2023-12-12 21:01:32,629] INFO 0ec0e507-5859-4d4f-9118-cbb6e8dedfc8 27 {'message': 'provider type not valid for subs processing', 'tracing_id': '1d5f8607-12ed-4ee4-8d1b-5566b7c0ef5a', 'schema': 'org1234567', 'provider_type': 'Azure', 'provider_uuid': '3b3b21da-03ca-414d-ab35-bdfa17f459b5'}
subs_worker  | [2023-12-12 21:01:32,629: INFO/ForkPoolWorker-1] {'message': 'provider type not valid for subs processing', 'tracing_id': '1d5f8607-12ed-4ee4-8d1b-5566b7c0ef5a', 'schema': 'org1234567', 'provider_type': 'Azure', 'provider_uuid': '3b3b21da-03ca-414d-ab35-bdfa17f459b5'}
subs_worker  | [2023-12-12 21:01:44,274] INFO 0ec0e507-5859-4d4f-9118-cbb6e8dedfc8 27 {'message': 'provider type not valid for subs processing', 'tracing_id': '413d821e-e225-4463-9666-b0d48e3a56ec', 'schema': 'org1234567', 'provider_type': 'Azure', 'provider_uuid': '3b3b21da-03ca-414d-ab35-bdfa17f459b5'}
subs_worker  | [2023-12-12 21:01:44,274: INFO/ForkPoolWorker-1] {'message': 'provider type not valid for subs processing', 'tracing_id': '413d821e-e225-4463-9666-b0d48e3a56ec', 'schema': 'org1234567', 'provider_type': 'Azure', 'provider_uuid': '3b3b21da-03ca-414d-ab35-bdfa17f459b5'}

...

@codecov
Copy link

codecov bot commented Oct 28, 2023

Codecov Report

Merging #4754 (a9e27af) into main (5fdf225) will decrease coverage by 0.0%.
The diff coverage is 90.8%.

Additional details and impacted files
@@           Coverage Diff           @@
##            main   #4754     +/-   ##
=======================================
- Coverage   94.0%   94.0%   -0.0%     
=======================================
  Files        364     364             
  Lines      30132   30204     +72     
  Branches    3589    3598      +9     
=======================================
+ Hits       28312   28379     +67     
- Misses      1160    1164      +4     
- Partials     660     661      +1     

Copy link
Contributor

@lcouzens lcouzens left a comment

Choose a reason for hiding this comment

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

Awesome job Corey!! I left you a few minor suggestions that we may need to handle. Otherwise it looks good! Well until Eva gets to work 🤣

koku/subs/subs_data_extractor.py Outdated Show resolved Hide resolved
koku/subs/subs_data_extractor.py Outdated Show resolved Hide resolved
koku/subs/subs_data_extractor.py Outdated Show resolved Hide resolved
koku/subs/subs_data_messenger.py Outdated Show resolved Hide resolved
client_secret = credentials.get("client_secret")
_factory = AzureClientFactory(subscription_id, tenant_id, client_id, client_secret)
compute_client = _factory.compute_client
response = compute_client.virtual_machines.get(
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm sure you already have this covered. But for my curiosity, does this client query need additional permissions exposed on the customer side? I assume it does since we only have something like storage blob and cost management reader roles right now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, there's an additional Reader role that will need to be added. It will be called out in docs

koku/subs/test/test_subs_data_extractor.py Outdated Show resolved Hide resolved
koku/subs/trino_sql/azure_subs_pre_or_clause.sql Outdated Show resolved Hide resolved
koku/subs/trino_sql/azure_subs_pre_or_clause.sql Outdated Show resolved Hide resolved
@cgoodfred cgoodfred self-assigned this Dec 8, 2023
@cgoodfred cgoodfred added the aws-smoke-tests pr_check will build the image and run aws + ocp on aws smoke tests label Dec 8, 2023
@cgoodfred cgoodfred marked this pull request as ready for review December 12, 2023 20:26
@cgoodfred cgoodfred requested review from a team as code owners December 12, 2023 20:26
djnakabaale
djnakabaale previously approved these changes Dec 12, 2023
Copy link
Contributor

@djnakabaale djnakabaale left a comment

Choose a reason for hiding this comment

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

lgtm! 👍🏾

djnakabaale
djnakabaale previously approved these changes Dec 12, 2023
Copy link

sonarcloud bot commented Dec 13, 2023

Quality Gate Passed Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

1 New issue
0 Security Hotspots
91.3% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@cgoodfred cgoodfred closed this Dec 13, 2023
@cgoodfred cgoodfred deleted the COST-3814-Azure-subs-processing branch July 12, 2024 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aws-smoke-tests pr_check will build the image and run aws + ocp on aws smoke tests smokes-required
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants