#8379 Add the functionality to generate report about the AMI purge in Pagure
Merged 4 years ago by dustymabe. Opened 4 years ago by sayanchowdhury.
sayanchowdhury/releng add-reporting-github  into  master

file modified
+167 -4
@@ -47,6 +47,7 @@ 

  import functools

  import fedfind

  import fedfind.release

+ import libpagure

  import requests

  

  from datetime import datetime, timedelta, date
@@ -60,6 +61,18 @@ 

  aws_access_key_id = os.environ.get("AWS_ACCESS_KEY")

  aws_secret_access_key = os.environ.get("AWS_SECRET_ACCESS_KEY")

  

+ pagure_access_token = os.environ.get("PAGURE_ACCESS_TOKEN")

+ instance_url = "https://stg.pagure.io/"

+ repo_name = "ami-purge-report"

+ #

+ # Make the connection to Pagure

+ project = libpagure.Pagure(pagure_token=pagure_access_token,

+                            pagure_repository=repo_name,

+                            instance_url=instance_url)

+ no_auth_project = libpagure.Pagure(

+                            pagure_repository=repo_name,

+                            instance_url=instance_url)

+ 

  DATAGREPPER_URL = "https://apps.fedoraproject.org/datagrepper/"

  NIGHTLY = "nightly"

  
@@ -213,6 +226,7 @@ 

      :args dry_run: dry run the flow

      """

      log.info("Deleting AMIs")

+     deleted_amis = []

      for region in REGIONS:

          log.info("%s Starting" % region)

          # Create a connection to an AWS region
@@ -232,6 +246,7 @@ 

          for ami in amis:

              try:

                  ami_id = ami["ImageId"]

+                 ami_name = ami['Name']

                  is_launch_permitted = ami["Public"]

                  _index = len(ami["BlockDeviceMappings"])

                  snapshot_id = ami["BlockDeviceMappings"][0]["Ebs"]["SnapshotId"]
@@ -265,11 +280,14 @@ 

                  if not dry_run:

                      conn.deregister_image(ImageId=ami_id)

                      conn.delete_snapshot(SnapshotId=snapshot_id)

+                     deleted_amis.append((ami_id, region, revoketimestamp, ami_name))

                  else:

                      print(ami_id)

              except Exception as ex:

                  log.error("%s: %s failed\n%s" % (region, ami_id, ex))

  

+     return deleted_amis

+ 

  

  def change_amis_permission_nd(amis, dry_run=False):

      """ Change the launch permissions of the AMIs to private.
@@ -282,6 +300,7 @@ 

      """

      log.info("Changing permission for AMIs")

      todaystimestamp = date.today().strftime("%d%m%Y")

+     changed_permission_amis = []

  

      for region in REGIONS:

          log.info("%s: Starting" % region)
@@ -296,6 +315,14 @@ 

  

          # Filter all the nightly AMIs belonging to this region

          r_amis = [(c, a, r) for c, a, r in amis if r == region]

+         amis_meta = conn.describe_images(

+                 ImageIds=[a for _, a, _ in amis]

+         )

+ 

+         ami_info = {}

+         for ami_meta in amis_meta:

+             image_id = ami_meta["ImageId"]

+             ami_info[image_id] = ami_meta['Name']

  

          # Loop through the AMIs change the permissions

          for _, ami_id, region in r_amis:
@@ -314,11 +341,143 @@ 

                              }

                          ],

                      )

+                     name = ami_info.get('Name', '')

+                     changed_permission_amis.append((ami_id, region, name))

                  else:

                      print(ami_id)

              except Exception as ex:

                  log.error("%s: %s failed \n %s" % (region, ami_id, ex))

  

+     return changed_permission_amis

+ 

+ 

+ def generate_report_changed_permission(dg_amis, pc_amis):

+     """ This is to generate the report via issue tracker in Pagure.

+ 

+     :args dg_amis: list of AMIs that was generated from Datagrepper

+     :args pc_amis: list of AMIs whose perms were actually changed.

+     """

+     log.info("Generating report for the day: Change Permissions")

+     todaystimestamp = date.today().strftime("%d%m%Y")

+     dg_output_string = []

+     pc_output_string = []

+ 

+     output_tmpl = """

+ {region}

+ ---

+ {amis}

+ """

+ 

+     for region in REGIONS:

+         r_amis = [a for c, a, r in dg_amis if r == region]

+         dg_output_string.append(output_tmpl.format(

+             region=region,

+             amis='\n'.join(r_amis)

+         ))

+ 

+     for region in REGIONS:

+         r_amis = ["%s - %s" % (a, n) for a, r, n in pc_amis if r == region]

+         pc_output_string.append(output_tmpl.format(

+             region=region,

+             amis='\n'.join(r_amis)

+         ))

+ 

+     dg_output_string = "\n\n".join(dg_output_string)

+     pc_output_string = "\n\n".join(pc_output_string)

+     content = "Report for the run on {todaystimestamp}".format(

+         todaystimestamp=todaystimestamp)

+ 

+     create_issue_params = {

+         'title': todaystimestamp,

+         'content': content,

+     }

+ 

+     try:

+         created_issue = project.create_issue(**create_issue_params)

+         created_issue_id = created_issue['issue']['id']

+     except Exception as ex:

+         log.error("There was an error creating issue: %s" % ex)

+         return

+ 

+     # Comment the details of the AMIs fetched from datagrepper

+     content = """

+ The list of AMIs that we fetch from Datagrepper

+ {dg_output_string}""".format(dg_output_string=dg_output_string)

+ 

+     dg_params = {

+         'issue_id': created_issue_id,

+         'body': content,

+     }

+     try:

+         project.comment_issue(**dg_params)

+     except Exception as ex:

+         log.error("There was an error creating issue: %s" % ex)

+ 

+     # Comment the details of the AMIs whose permissions were changed.

+     content = """

+ The list of AMIs whose permissions were changed.

+ {pc_output_string}""".format(pc_output_string=pc_output_string)

+ 

+     pc_params = {

+         'issue_id': created_issue_id,

+         'body': content,

+     }

+ 

+     try:

+         project.comment_issue(**pc_params)

+     except Exception as ex:

+         log.error("There was an error creating issue: %s" % ex)

+ 

+ 

+ def generate_report_delete(dl_amis):

+     """ This is to generate the report via issue tracker in Pagure.

+ 

+     :args dl_amis: list of AMIs that were deleted

+     """

+     log.info("Generating report for the day: Delete")

+     todaystimestamp = date.today().strftime("%d%m%Y")

+     created_issue_id = None

+     dl_output_string = []

+ 

+     output_tmpl = """

+ {region}

+ ---

+ {amis}

+ """

+ 

+     for region in REGIONS:

+         r_amis = ["%s - %s - %s" % (a, n, t) for a, r, t, n in dl_amis if r == region]

+         dl_output_string.append(output_tmpl.format(

+             region=region,

+             amis='\n'.join(r_amis)

+         ))

+ 

+     dl_output_string = "\n\n".join(dl_output_string)

+ 

+     issues = no_auth_project.list_issues()

+     for issue in issues:

+         if issue['title'] == todaystimestamp:

+             created_issue_id = issue['id']

+             break

+ 

+     if created_issue_id is None:

+         log.info("We could not find the request issue: %s" % todaystimestamp)

+         return

+ 

+     # Comment the details of the AMIs which were deleted

+     content = """

+ The list of AMIs that were deleted.

+ {dl_output_string}""".format(dl_output_string=dl_output_string)

+ 

+     dl_params = {

+         'issue_id': created_issue_id,

+         'body': content,

+     }

+     try:

+         project.comment_issue(**dl_params)

+     except Exception as ex:

+         log.error("There was an error creating issue: %s" % ex)

+ 

  

  if __name__ == "__main__":

      argument_parser = argparse.ArgumentParser()
@@ -393,12 +552,16 @@ 

              )

          end = (datetime.now() - timedelta(days=permswaitperiod)).strftime("%s")

          amis = _get_nightly_amis_nd(

-             delta=86400 * (days - permswaitperiod), end=int(end)

+            delta=86400 * (days - permswaitperiod), end=int(end)

          )

-         change_amis_permission_nd(amis, dry_run=args.dry_run)

+         perms_changed_amis = change_amis_permission_nd(amis, dry_run=args.dry_run)

+         if not args.dry_run:

+             generate_report_changed_permission(amis, perms_changed_amis)

  

      if args.delete:

          deletetimestamp = (

-             datetime.now() - timedelta(days=deletewaitperiod)

+            datetime.now() - timedelta(days=deletewaitperiod)

          ).strftime("%d%m%Y")

-         delete_amis_nd(deletetimestamp, dry_run=args.dry_run)

+         deleted_amis = delete_amis_nd(deletetimestamp, dry_run=args.dry_run)

+         if not args.dry_run:

+             generate_report_delete(deleted_amis)

This is the to generate a report of the AMIs purge that would be happening in Pagure. This is so that we can monitor the status of the uploads.

Signed-off-by: Sayan Chowdhury sayan.chowdhury2012@gmail.com

this is so we track via pagure what images were deleted?

rebased onto 549615c4de030180e3c6fbd8d5f0b880df4b4398

4 years ago

this is so we track via pagure what images were deleted?

Yes, also which images are getting deleted, and whose permission are getting revoked.

is there no way to get the created issue id from this call? instead of having to discover it by searching for the issue title like you do below? if we keep it like this created_issue is never used I don't think

is there a reason to use comments vs just added all the data to the original description of the opened issue ? Do you have a repo set up where you are already opening these reports?

on a dry run we should maybe generate a report and just print it to the screen? or maybe we should always print the report to the screen, or is it super lengthy?

Yes, created_issue is right now not in use. Technically the API should be returning the response given here https://pagure.io/api/0/#issues , but I'm instead getting None. That's why I'm searching through the titles :(

I planned to add it to the description but later moved to comments because I thought it would be more clearer. Also, lesser chances of hitting the max limit on the field if there is any.

for dry run, I would be +1 for just printing it to screen.

it would be great if in the report we could also get the description of the AMI like Fedora-Atomic-Host-20190122.blah.blah in cases where we have that information

rebased onto ee222caba48767993c5ffddb9c9be6c995b308bf

4 years ago

rebased onto 763cac9fc508516636f6b97e3feaac2d973f2e8e

4 years ago

We discussed last week not using comments in the issue but rather a better description and an attached file (for the long list of AMIs). Did you investigate this route?

for dry run, I would be +1 for just printing it to screen.

The new code doesn't seem to be calling those functions at all for dry_run, which means it won't print to the screen.

Yes, created_issue is right now not in use. Technically the API should be returning the response given here https://pagure.io/api/0/#issues , but I'm instead getting None. That's why I'm searching through the titles :(

This doesn't seem to be addressed

rebased onto da2001f410e9b701161a477ae145c6065bb25f1f

4 years ago

rebased onto 96e70fb852ebe2994589bb4446d6dca830055b18

4 years ago

We discussed last week not using comments in the issue but rather a better description and an attached file (for the long list of AMIs). Did you investigate this route?

I did look into this but Pagure does not support attaching files.

for dry run, I would be +1 for just printing it to screen.

The new code doesn't seem to be calling those functions at all for dry_run, which means it won't print to the screen.

I meant, dry run can be used only when we want to test the output of the script. It still prints the value of the AMIs. The given two lines would call the functions that would print the ami ids during the dry run
- https://pagure.io/releng/pull-request/8379#_1__244
- https://pagure.io/releng/pull-request/8379#_1__236

Yes, created_issue is right now not in use. Technically the API should be returning the response given here https://pagure.io/api/0/#issues , but I'm instead getting None. That's why I'm searching through the titles :(

This doesn't seem to be addressed

Oh! this one got skipped. I've addressed this now - https://pagure.io/releng/pull-request/8379#_1__139

We discussed last week not using comments in the issue but rather a better description and an attached file (for the long list of AMIs). Did you investigate this route?

I did look into this but Pagure does not support attaching files.

weird - do you mind openeing an RFE under pagure.io/pagure repo so we can use it in the future?

for dry run, I would be +1 for just printing it to screen.
The new code doesn't seem to be calling those functions at all for dry_run, which means it won't print to the screen.

I meant, dry run can be used only when we want to test the output of the script. It still prints the value of the AMIs. The given two lines would call the functions that would print the ami ids during the dry run
- https://pagure.io/releng/pull-request/8379#_1__244
- https://pagure.io/releng/pull-request/8379#_1__236

I think those line numbers got off with the new upload. Where are the print statements? is this something that was pre-existing before this PR?

Yes, created_issue is right now not in use. Technically the API should be returning the response given here https://pagure.io/api/0/#issues , but I'm instead getting None. That's why I'm searching through the titles :(
This doesn't seem to be addressed

Oh! this one got skipped. I've addressed this now - https://pagure.io/releng/pull-request/8379#_1__139

ok it looks better now. I wish we didn't need to use the following logic in the delete code but since it runs as a separate process I think it's probably necessary:

    issues = no_auth_project.list_issues()   
    for issue in issues:                     
        if issue['title'] == todaystimestamp:
            created_issue_id = issue['id']   
            break                            

weird - do you mind openeing an RFE under pagure.io/pagure repo so we can use it in the future?

Sure, I will do that.

I think those line numbers got off with the new upload. Where are the print statements? is this something that was pre-existing before this PR?

https://pagure.io/fork/sayanchowdhury/releng/blob/c0f1304650b258e2a9dccca2c7165d9bc608ae05/f/scripts/clean-amis.py#_285
https://pagure.io/fork/sayanchowdhury/releng/blob/c0f1304650b258e2a9dccca2c7165d9bc608ae05/f/scripts/clean-amis.py#_347

dry-run has been there from the beginning of this script.

yeah dry run has been there. I didn't know if you wanted a nicely formatted report to be output during dry-run as well or not. If not, that's perfectly fine :)

I just print the amis' as it is easier for me to use. I don't need to need to do much string processing of the output incase I need to do something.

Maybe I can create a seperate PR for formatting, if needed.

no need - this PR LGTM, we can incrementally make it better as we go.

have you tested this? Are you ready for merge?

rebased onto e1a6b5c

4 years ago

no need - this PR LGTM, we can incrementally make it better as we go.
have you tested this? Are you ready for merge?

Rebased. Should be good to merge now.

Yes, I have tested it.

Pull-Request has been merged by dustymabe

4 years ago
Metadata