Merge bootstack-ops:juju-select-largest-model into bootstack-ops:master

Proposed by David O Neill
Status: Rejected
Rejected by: Giuseppe Petralia
Proposed branch: bootstack-ops:juju-select-largest-model
Merge into: bootstack-ops:master
Diff against target: 290 lines (+234/-6) (has conflicts)
1 file modified
bootstack-ops/cloud_report.py (+234/-6)
Conflict in bootstack-ops/cloud_report.py
Reviewer Review Type Date Requested Status
Giuseppe Petralia Disapprove
Zachary Zehring (community) Needs Fixing
Drew Freiberger (community) Needs Fixing
Review via email: mp+366113@code.launchpad.net

Commit message

Switches the largest juju model prior to other juju commands

To post a comment you must log in.
Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

Revision history for this message
Giuseppe Petralia (peppepetra) wrote :

Comments inline

Revision history for this message
Giuseppe Petralia (peppepetra) :
review: Needs Fixing
Revision history for this message
Drew Freiberger (afreiberger) wrote :

comments inline

Revision history for this message
Paul Goins (vultaire) :
Revision history for this message
Paul Goins (vultaire) :
Revision history for this message
Drew Freiberger (afreiberger) wrote :

merge conflict in the code

review: Needs Fixing
Revision history for this message
Zachary Zehring (zzehring) wrote :

inline comments. have some more but will save when merge conflict is resolved.

review: Needs Fixing
Revision history for this message
Andrea Ieri (aieri) wrote :

I agree with Paul, we have to take into account clouds that have multiple models.
Aside from CDK clusters (which may or may not need to be billed), we also have clouds that use separate models for ceph backup clusters.
We should perhaps think about {white,black}listing models instead.
See also https://bugs.launchpad.net/bootstack-ops/+bug/1842356

Revision history for this message
Giuseppe Petralia (peppepetra) wrote :
review: Disapprove

Unmerged commits

2b622cd... by David O Neill

Switches the largest juju model prior to other juju commands

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/bootstack-ops/cloud_report.py b/bootstack-ops/cloud_report.py
index f13a5d3..d278db5 100644
--- a/bootstack-ops/cloud_report.py
+++ b/bootstack-ops/cloud_report.py
@@ -43,6 +43,8 @@ import json
43import click43import click
44import subprocess44import subprocess
45import yaml45import yaml
46import re
47
4648
47from maas.client import login, connect49from maas.client import login, connect
48from flask import Flask, jsonify50from flask import Flask, jsonify
@@ -382,22 +384,38 @@ class Juju(object):
382 def disconnect(self):384 def disconnect(self):
383 pass385 pass
384386
385 def _list_machines(self):387 def run_juju_command(self, command, **kwargs):
388 """
389 Run a juju command with args and specify the output format
390
391 :param command: Juju command
392 :param format: the output format, tabulaur (None), json, yaml
393 :param commandargs: argument for the specific command
394 :return: string or None
395 """
386 if not JUJU_DATA:396 if not JUJU_DATA:
387 print("Please export JUJU_DATA env variable before running the script")397 print("Please export JUJU_DATA env variable before running the script")
388 sys.exit(1)398 sys.exit(1)
389399
390 machines = []
391 env = os.environ.copy()400 env = os.environ.copy()
392 env["JUJU_DATA"] = JUJU_DATA401 env["JUJU_DATA"] = JUJU_DATA
393402
403 format = "--format=" + kwargs['format'] if "format" in kwargs else None
404
394 if os.path.isfile('/snap/bin/juju'):405 if os.path.isfile('/snap/bin/juju'):
395 cmd = ['/snap/bin/juju', 'status', '--format=yaml']406 cmd = ['/snap/bin/juju', command]
396 elif os.path.isfile("/usr/bin/juju"):407 elif os.path.isfile("/usr/bin/juju"):
397 cmd = ['/usr/bin/juju', 'status', '--format=yaml']408 cmd = ['/usr/bin/juju', command]
398 else:409 else:
399 cmd = ['snap run juju', 'status', '--format=yaml']410 cmd = ['snap run juju', command]
400411
412 if "commandargs" in kwargs:
413 cmd.append(kwargs['commandargs'])
414
415 if format is not None:
416 cmd.append(format)
417
418<<<<<<< bootstack-ops/cloud_report.py
401 for i in range(0, RETRY):419 for i in range(0, RETRY):
402 try:420 try:
403 output = subprocess.check_output(cmd, timeout=30, env=env)421 output = subprocess.check_output(cmd, timeout=30, env=env)
@@ -409,6 +427,114 @@ class Juju(object):
409 time.sleep(SLEEP * i)427 time.sleep(SLEEP * i)
410 428
411 return machines429 return machines
430=======
431 try:
432 return subprocess.check_output(cmd, env=env)
433 except subprocess.CalledProcessError as e:
434 print("Exception calling juju status command. {}".format(e))
435 # implicit, but lets do it anyway
436 return None
437
438
439 def _list_machines(self):
440 res = self.run_juju_command("status", format="yaml")
441
442 if res is None:
443 print("Failed to get machine list, @see run_juju_command")
444 return None
445
446 return yaml.safe_load(res)
447
448
449 def switch_model(self, model):
450 """
451 Switches to a given model
452 :param model: the model to switch to
453 """
454
455 res = self.run_juju_command("switch", commandargs=model)
456
457 if res is None:
458 print("Exception calling juju switch, @see run_juju_command")
459 return None
460
461 def list_models(self):
462 """
463 Gets the output of juju list-models
464 :returns models: string of the command output
465 """
466
467 models = self.run_juju_command("list-models")
468
469 if models is None:
470 print("Exception calling juju list-models command, @see run_juju_command")
471 return None
472
473 return models
474
475 def get_model_and_sizes(self):
476 """
477 From the output of juju list-models we create a dict of the format
478 {
479 'model-name': machine-count
480 'model-name': machine-count
481 'model-name': machine-count
482 }
483 :returns dict: the above format
484 """
485 models = self.list_models().decode('utf8').split('\n')
486 modelsdict = {}
487 machinescolnum = 0
488
489 for line in models:
490 if line.strip() == "":
491 continue
492
493 if line.startswith('Controller') and machinescolnum == 0:
494 continue
495
496 line = line.replace('\n', ' ').replace('\r', '')
497 cols = re.split('\s{2,100}', line)
498
499 if machinescolnum == 0:
500 for col in cols:
501 if col == "Machines":
502 break
503 machinescolnum += 1
504 else:
505 modelsdict[cols[0]] = int(cols[machinescolnum])
506
507 return modelsdict
508
509 def switch_to_largest_model(self):
510 """
511 Looks through a dict of models in format
512 {
513 'model-name': machine-count
514 'model-name': machine-count
515 'model-name': machine-count
516 }
517 Determiens the largest model from the machine count
518 Then calls switch_model
519 """
520 largest = -1
521 largestModel = None
522 models = self.get_model_and_sizes()
523
524 if len(models) == 0:
525 print("No models found ?????")
526 return
527
528 for k, v in models.items():
529 if v > largest:
530 largest = v
531 largestModel = k
532
533 if "*" not in largestModel:
534 self.switch_model(largestModel)
535 else:
536 print("Largest model already selected: " + largestModel)
537>>>>>>> bootstack-ops/cloud_report.py
412538
413 def connect(self):539 def connect(self):
414 return True540 return True
@@ -478,6 +604,108 @@ class Juju(object):
478604
479 return m_json605 return m_json
480606
607 def switch_model(self, model):
608 """
609 Switches to a given model
610 :param model: the model to switch to
611 """
612
613 if os.path.isfile('/snap/bin/juju'):
614 cmd = ['/snap/bin/juju', 'switch', model]
615 elif os.path.isfile("/usr/bin/juju"):
616 cmd = ['/usr/bin/juju', 'switch', model]
617 else:
618 cmd = ['snap run juju', 'switch', model]
619
620 try:
621 subprocess.check_output(cmd)
622 except subprocess.CalledProcessError as e:
623 print("Exception calling juju status command. {}".format(e))
624
625 def list_models(self):
626 """
627 Gets the output of juju list-models
628 :returns dict: the above format
629 """
630 models = None
631
632 if os.path.isfile('/snap/bin/juju'):
633 cmd = ['/snap/bin/juju', 'list-models']
634 elif os.path.isfile("/usr/bin/juju"):
635 cmd = ['/usr/bin/juju', 'list-models']
636 else:
637 cmd = ['snap run juju', 'list-models']
638
639 try:
640 models = subprocess.check_output(cmd).decode("utf-8")
641 except subprocess.CalledProcessError as e:
642 print("Exception calling juju status command. {}".format(e))
643
644 return models
645
646 def get_model_and_sizes(self):
647 """
648 From the output of juju list-models we create a dict of the format
649 {
650 'model-name': machine-count
651 'model-name': machine-count
652 'model-name': machine-count
653 }
654 :returns dict: the above format
655 """
656 models = self.list_models().split('\n')
657 modelsDict = {}
658 machinesColnum = 0
659
660 for line in models:
661 if line.strip() == "":
662 continue
663
664 if line.startswith('Controller') and machinesColnum == 0:
665 continue
666
667 line = line.replace('\n', ' ').replace('\r', '')
668 cols = re.split('\s{2,100}', line)
669
670 if machinesColnum == 0:
671 for col in cols:
672 if col == "Machines":
673 break
674 machinesColnum += 1
675 else:
676 modelsDict[cols[0]] = int(cols[machinesColnum])
677
678 return modelsDict
679
680 def switch_to_largest_model(self):
681 """
682 Looks through a dict of models in format
683 {
684 'model-name': machine-count
685 'model-name': machine-count
686 'model-name': machine-count
687 }
688 Determiens the largest model from the machine count
689 Then calls switch_model
690 """
691 largest = -1
692 largestModel = None
693 models = self.get_model_and_sizes()
694
695 if len(models) == 0:
696 print("No models found ?????")
697 return
698
699 for k,v in models.items():
700 if v > largest:
701 largest = v
702 largestModel = k
703
704 if "*" not in largestModel:
705 self.switch_model(largestModel)
706 else:
707 print("Largest model already selected: " + largestModel)
708
481 def get_report(self, running=False):709 def get_report(self, running=False):
482 """710 """
483 Get the json report for running machines retrieved using Juju cli.711 Get the json report for running machines retrieved using Juju cli.
@@ -485,7 +713,7 @@ class Juju(object):
485 :param running: If it is set to true only machines with status 'running' will be added to the report713 :param running: If it is set to true only machines with status 'running' will be added to the report
486 :return: json report714 :return: json report
487 """715 """
488716 self.switch_to_largest_model()
489 machines = self._list_machines()717 machines = self._list_machines()
490 juju_machines = []718 juju_machines = []
491 for m in machines:719 for m in machines:

Subscribers

People subscribed via source and target branches

to all changes: