Lock cloud machines

[moishce]

Status

• In Progress
• Ready
• In Hold - (Reason for hold)

Related Issues

fixes: link to the issue

Description

Rewrite the “Lock XSIAM machines script” to Python according to this flowchart

Screenshots

Paste here any images that will help the reviewer

Minimum version of Cortex XSOAR

• 6.0.0
• 6.1.0
• 6.2.0
• 6.5.0

Does it break backward compatibility?

• Yes
  • Further details:
• No

Must have

• Tests
• Documentation

[ShahafBenYakir]

Very good work!

1. Go over all functions and make sure the docstring is good (in some we are missing arguments documentation and in others, the function explanation is missing/poor)
2. I believe that when there are several confusing arguments, it is a better practice to call a function using the named parameters. For example, do this: get_queue_locks_details(storage_client=storage_client, bucket_name='xsoar-ci-artifacts', prefix=f'{gcs_locks_path}/{QUEUE_REPO}/') instead of this: get_queue_locks_details(storage_client, 'xsoar-ci-artifacts', f'{gcs_locks_path}/{QUEUE_REPO}/'). Let's go over the code and functions and add the named parameters, where they add more clarity.
3. Great work on adding intensive logging. I think we need to differentiate between info and debug logs such that info logs are for general operations so that the user can easily understand what's his stus, whereas debug logs are for more atomic operations which are usually less interesting unless I want to debug the process. For example, the following should be info logs:

   'Adding job_id to the queue'
   'Waiting for my turn in the queue'
   'My place in queue is 2'
   'My place in queue is 1'
   'starting to search for an empty machine'
   

   debug logs would be:

   'getting all builds in the queue'
   'There is a lock file for job id: 1234'
   'There is no a lock file for machine: bla bla'
   

4. After adding the two functions I suggested in the main(), I want to add several use cases in the unit tests, (we can add mocking to the parts that were tested already by other unit tests if you think it is better)

[daryakoval]

Are we have the option to lock more than one machine for the build?
In XSOAR we use more than one machine in the nightly, maybe we will want in the future to add the same functionality to XSOAR-NG.

[moishce]

@daryakoval Yes, the script has the option to lock several machines for a job.

[ShahafBenYakir]

Great! How can we test?

[daryakoval]

Are you iterating for all these folders?
If yes why?

[moishce]

I have not found another way that works in our Becket.
(there is an option to do it through an API call, which I will check)
In our case, our files are at the beginning of the list, so it doesn't actually go through the entire bucket (but it's obviously not the best way)

[ShahafBenYakir]

Why not putting both content-locks-xsiam and content-locks-xsoar-ng in content-locks?
also when we are done we need to delete the test folder.

[daryakoval]

It will be easier to debug if we will have different folders.

[ShahafBenYakir]

I didn't say NOT to have different folders, I said to put them both under content-locks. It should be separated from other artifacts (tests, content, etc...)

[ShahafBenYakir]

Great work, please address this
@daryakoval please review the fixes you requested and approve once you are ok.

[daryakoval]

Great!