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

Processes the index projects in batches #612

Merged
merged 10 commits into from Sep 17, 2018

Adds 120 seconds delay after processing each batch

  • Loading branch information...
navidshaikh committed Sep 5, 2018
commit d11a5f5188490a8f98d92e0b9999d542e3a3ae69
View
@@ -19,7 +19,6 @@ class InvalidPipelineName(Exception):
"""
pass
class ErrorAccessingIndexEntryAttributes(Exception):
"""
Exception to be raised when there are errors accessing
@@ -28,6 +27,14 @@ class ErrorAccessingIndexEntryAttributes(Exception):
pass
def _print(msg):
"""
_prints the given msg
"""
print (msg)
sys.stdout.flush()

This comment has been minimized.

@bamachrn

bamachrn Sep 11, 2018

Collaborator

we added the stdout flush to make sure jenkins is not holding them back from printing right?

@bamachrn

bamachrn Sep 11, 2018

Collaborator

we added the stdout flush to make sure jenkins is not holding them back from printing right?

This comment has been minimized.

@navidshaikh

navidshaikh Sep 11, 2018

Collaborator

That's correct.

@navidshaikh

navidshaikh Sep 11, 2018

Collaborator

That's correct.

def run_cmd(cmd, shell=False):
"""
Runs the given shell command
@@ -189,7 +196,7 @@ def read_yaml(self, filepath):
with open(filepath) as fin:
data = yaml.load(fin, Loader=yaml.BaseLoader)
except yaml.YAMLError as exc:
print ("Failed to read {}".format(filepath))
_print("Failed to read {}".format(filepath))
raise(exc)
else:

This comment has been minimized.

@bamachrn

bamachrn Sep 11, 2018

Collaborator

how are we handling this raise here we are raising exception from exception?

@bamachrn

bamachrn Sep 11, 2018

Collaborator

how are we handling this raise here we are raising exception from exception?

This comment has been minimized.

@navidshaikh

navidshaikh Sep 11, 2018

Collaborator

We are raising the exception but ensuring we print error message along with filepath with issue.

@navidshaikh

navidshaikh Sep 11, 2018

Collaborator

We are raising the exception but ensuring we print error message along with filepath with issue.

This comment has been minimized.

@navidshaikh

navidshaikh Sep 11, 2018

Collaborator

We have not handled this. Index Reader expects the yaml files to be in sane format. Do we want to log error and proceed further in this case as well?

@navidshaikh

navidshaikh Sep 11, 2018

Collaborator

We have not handled this. Index Reader expects the yaml files to be in sane format. Do we want to log error and proceed further in this case as well?

This comment has been minimized.

@bamachrn

bamachrn Sep 12, 2018

Collaborator

yeah, these should be taken care of by the index CI. But index reader should not fail entirely for single malformed entry. I would suggest we log it and continue to process other entries.

@bamachrn

bamachrn Sep 12, 2018

Collaborator

yeah, these should be taken care of by the index CI. But index reader should not fail entirely for single malformed entry. I would suggest we log it and continue to process other entries.

This comment has been minimized.

@navidshaikh

navidshaikh Sep 12, 2018

Collaborator

Agreed. I'll update the PR.

@navidshaikh

navidshaikh Sep 12, 2018

Collaborator

Agreed. I'll update the PR.

return data
@@ -212,13 +219,12 @@ def read_projects(self):
try:
project = Project(entry, self.namespace)
except Exception as e:
print("Error processing index entry {}. Moving on.".format(
_print("Error processing index entry {}. Moving on.".format(
entry))
print("Error: {}".format(e))
_print("Error: {}".format(e))
else:
# append to the list of projects
projects.append(project)
return projects
@@ -315,14 +321,14 @@ def apply_build_job(self,
)
# process and apply buildconfig
output = run_cmd(command, shell=True)
print (output)
_print(output)
# if a buildConfig has config update, oc apply returns
# "buildconfig.build.openshift.io "$PIPELINE_NAME" configured"
# possible values are ["unchanged", "created", "configured"]
# we are looking for "configured" string for updated pipelines
if "configured" in output:
print ("{} is updated, starting build..".format(
_print("{} is updated, starting build..".format(
project.pipeline_name))
self.start_build(project.pipeline_name)
@@ -361,7 +367,7 @@ def apply_weekly_scan(self,
)
# process and apply buildconfig
output = run_cmd(command, shell=True)
print (output)
_print(output)
def apply_buildconfigs(self,
project,
@@ -380,15 +386,15 @@ def start_build(self, pipeline_name):
"""
command = "oc start-build {} -n {}".format(
pipeline_name, self.namespace)
print (run_cmd(command))
_print(run_cmd(command))
def delete_buildconfigs(self, bcs):
"""
Deletes the given list of bcs
"""
command = "oc delete -n {} bc {}"
for bc in bcs:
print ("Deleting buildConfig {}".format(bc))
_print("Deleting buildConfig {}".format(bc))
run_cmd(command.format(self.namespace, bc))
def list_all_builds(self):
@@ -463,7 +469,7 @@ def run(self):
# list all jobs in index, list of project objects
index_projects = self.index_reader.read_projects()
print ("Number of projects in index {}".format(len(index_projects)))
_print("Number of projects in index {}".format(len(index_projects)))
# list existing jobs in openshift
oc_projects = self.bc_manager.list_all_buildConfigs()
@@ -473,7 +479,7 @@ def run(self):
oc_projects = [bc.split("/")[1] for bc in oc_projects
if bc.split("/")[1] not in self.infra_projects]
print ("Number of projects in OpenShift {}".format(len(oc_projects)))
_print("Number of projects in OpenShift {}".format(len(oc_projects)))
# names of pipelines for all projects in container index
index_project_names = [project.pipeline_name for project in
@@ -490,12 +496,12 @@ def run(self):
)
if stale_projects:
print ("List of stale projects:\n{}".format(
_print("List of stale projects:\n{}".format(
"\n".join(stale_projects)))
# delete all the stal projects/buildconfigs
self.bc_manager.delete_buildconfigs(stale_projects)
print ("Number of projects to be updated/created: {}".format(
_print("Number of projects to be updated/created: {}".format(
len(index_projects)))
self.batch_process_projects(index_projects)
@@ -515,7 +521,9 @@ def batch_process_projects(self,
in batches and oc apply the weekly scan jobs at the end
"""

This comment has been minimized.

@dharmit

dharmit Sep 16, 2018

Collaborator

👍 for clear comments about the scope of function.

@dharmit

dharmit Sep 16, 2018

Collaborator

👍 for clear comments about the scope of function.

# Split the projects to process in equal sized chunks
for batch in self.batch(index_projects, batch_size):
generator_obj = self.batch(index_projects, batch_size)
for batch in generator_obj:
outstanding_builds = True
while outstanding_builds:
# Check if builds are in status.phase other than Complete
@@ -526,37 +534,43 @@ def batch_process_projects(self,
filter_builds=self.infra_projects)

This comment has been minimized.

@dharmit

dharmit Sep 16, 2018

Collaborator

IIUC, we're executing a shell command to get output here. run_cmd and list_builds_except don't seem to have exception handling. So, if fetching outstanding builds fails due to some issue (like failure in executing oc command), seed-job would fail. Correct me if I'm wrong.

If I'm correct, I think this part is deep into the execution of our service. We could give a few retries to the scenario where oc fails to get our desired task done and fail the seed-job appropriate message if it fails even after the retries.

Thoughts @navidshaikh @bamachrn @mohammedzee1000 ?

@dharmit

dharmit Sep 16, 2018

Collaborator

IIUC, we're executing a shell command to get output here. run_cmd and list_builds_except don't seem to have exception handling. So, if fetching outstanding builds fails due to some issue (like failure in executing oc command), seed-job would fail. Correct me if I'm wrong.

If I'm correct, I think this part is deep into the execution of our service. We could give a few retries to the scenario where oc fails to get our desired task done and fail the seed-job appropriate message if it fails even after the retries.

Thoughts @navidshaikh @bamachrn @mohammedzee1000 ?

This comment has been minimized.

@navidshaikh

navidshaikh Sep 17, 2018

Collaborator

I think it makes sense - We can keep a retry cycle of 30 seconds with timeout limit set to 5 minutes ? And fail it is 5 minutes (10 retries) are lapsed ?

@navidshaikh

navidshaikh Sep 17, 2018

Collaborator

I think it makes sense - We can keep a retry cycle of 30 seconds with timeout limit set to 5 minutes ? And fail it is 5 minutes (10 retries) are lapsed ?

This comment has been minimized.

@navidshaikh

navidshaikh Sep 17, 2018

Collaborator

It makes sense. It applies to all the operations performed via oc. Adding a retry decorator which can be re-used and have it applied to al oc related operations.

@navidshaikh

navidshaikh Sep 17, 2018

Collaborator

It makes sense. It applies to all the operations performed via oc. Adding a retry decorator which can be re-used and have it applied to al oc related operations.

if outstanding_builds:
print ("Waiting for completion of builds {}".format(
outstanding_builds))
_print("Waiting for completion of builds {}".format(
outstanding_builds))
time.sleep(poll_cycle)
print ("Processing projects batch: {}".format(
_print("Processing projects batch: {}".format(
[each.pipeline_name for each in batch]))
for project in batch:
# oc process and oc apply to all fresh and existing jobs
try:
self.bc_manager.apply_build_job(project)
except Exception as e:
print("Error applying/creating build config for {}. "
"Moving on.".format(project.pipeline_name))
print("Error: {}".format(str(e)))
_print("Error applying/creating build config for {}. "
"Moving on.".format(project.pipeline_name))
_print("Error: {}".format(str(e)))
else:
# sleep for $poll_cycle seconds after processing each batch
# to have them appeared on the console
_print("Waiting for {} seconds to process current "
"batch".format(poll_cycle))
time.sleep(poll_cycle)
print ("Processing weekly scan projects..")
_print("Processing weekly scan projects..")
for project in index_projects:
try:
self.bc_manager.apply_weekly_scan(project)
except Exception as e:
print("Error applying/creating weekly scan build config "
"for {}. Moving on.".format(project.pipeline_name))
print("Error: {}".format(str(e)))
_print("Error applying/creating weekly scan build config "
"for {}. Moving on.".format(project.pipeline_name))
_print("Error: {}".format(str(e)))
else:
time.sleep(5)
if __name__ == "__main__":
if len(sys.argv) != 6:
print ("Incomplete set of input variables, please refer README.")
_print("Incomplete set of input variables, please refer README.")
sys.exit(1)
index = sys.argv[1].strip()
ProTip! Use n and p to navigate between commits in a pull request.