diff --git a/.argo/argo-platform-unit-tests.yaml b/.argo/argo-platform-unit-tests.yaml index a158293446a1..f5fccb768c88 100644 --- a/.argo/argo-platform-unit-tests.yaml +++ b/.argo/argo-platform-unit-tests.yaml @@ -16,11 +16,10 @@ steps: pytest -vv /src/platform/tests/kubeobject/ && pytest -vv /src/platform/tests/util/ && pytest -vv /src/platform/tests/lib/ax/platform/ax_asg_test.py && - pytest -vv /src/platform/tests/axmon/operations_test.py - - # TODO (#263): add those tests back later - # python2 -m pytest -vv /src/platform/tests/minion_manager/ax_minion_manager_test.py && - # python2 -m pytest -vv /src/platform/tests/minion_manager/ax_bid_advisor_test.py + pytest -vv /src/platform/tests/axmon/operations_test.py && + python2 -m pytest -s -vv /src/platform/tests/minion_manager/aws/aws_minion_manager_test.py && + python2 -m pytest -s -vv /src/platform/tests/minion_manager/aws/aws_bid_advisor_test.py && + python2 -m pytest -s -vv /src/platform/tests/minion_manager/broker_test.py KUBE-MONITOR-TESTS: template: argo-platform-unit-test-base arguments: diff --git a/platform/source/lib/ax/platform/minion_manager/cloud_provider/aws/aws_minion_manager.py b/platform/source/lib/ax/platform/minion_manager/cloud_provider/aws/aws_minion_manager.py index 89f6886a1fcb..719719173cca 100644 --- a/platform/source/lib/ax/platform/minion_manager/cloud_provider/aws/aws_minion_manager.py +++ b/platform/source/lib/ax/platform/minion_manager/cloud_provider/aws/aws_minion_manager.py @@ -81,8 +81,6 @@ def __init__(self, scaling_groups, region, **kwargs): name="MinionManagerRestAPI") self.rest_thread.setDaemon(True) - self.k8s_client = KubernetesApiClient() - @staticmethod @retry(wait_exponential_multiplier=1000, stop_max_attempt_number=3) def describe_asg_with_retries(ac_client, asgs): @@ -501,6 +499,7 @@ def get_asg_metas(self): def rest_api(self): """ Thread that responds to the Flask api endpoints. """ + k8s_client = KubernetesApiClient() app = Flask("MinionManagerRestAPI") def _update_config_map(enabled_str, asgs): @@ -510,7 +509,7 @@ def _update_config_map(enabled_str, asgs): if asgs: cmap.data["MM_SCALING_GROUPS"] = asgs - self.k8s_client.api.replace_namespaced_config_map( + k8s_client.api.replace_namespaced_config_map( cmap, MM_CONFIG_MAP_NAMESPACE, MM_CONFIG_MAP_NAME) @app.route('/spot_instance_config', methods=['PUT']) @@ -537,7 +536,7 @@ def _update_spot_instances(): @app.route('/spot_instance_config', methods=['GET']) def _get_spot_instances(): """ Get spot-instances config. """ - cmap = self.k8s_client.api.read_namespaced_config_map( + cmap = k8s_client.api.read_namespaced_config_map( namespace=MM_CONFIG_MAP_NAMESPACE, name=MM_CONFIG_MAP_NAME) return jsonify({"status": cmap.data["MM_SPOT_INSTANCE_ENABLED"], "asgs": cmap.data["MM_SCALING_GROUPS"]}) diff --git a/platform/tests/minion_manager/aws/aws_bid_advisor_test.py b/platform/tests/minion_manager/aws/aws_bid_advisor_test.py new file mode 100644 index 000000000000..05b3bd77b005 --- /dev/null +++ b/platform/tests/minion_manager/aws/aws_bid_advisor_test.py @@ -0,0 +1,128 @@ +"""The file has unit tests for the AWSBidAdvisor.""" + +import unittest + +from ax.platform.minion_manager.cloud_provider.aws.aws_bid_advisor import AWSBidAdvisor + +REFRESH_INTERVAL = 10 +REGION = 'us-west-2' + + +class AWSBidAdvisorTest(unittest.TestCase): + """ + Tests for AWSBidAdvisor. + """ + def test_ba_lifecycle(self): + """ + Tests that the AWSBidVisor starts threads and stops them correctly. + """ + bidadv = AWSBidAdvisor(REFRESH_INTERVAL, REFRESH_INTERVAL, REGION) + assert len(bidadv.all_bid_advisor_threads) == 0 + bidadv.run() + assert len(bidadv.all_bid_advisor_threads) == 2 + bidadv.shutdown() + assert len(bidadv.all_bid_advisor_threads) == 0 + + def test_ba_on_demand_pricing(self): + """ + Tests that the AWSBidVisor correctly gets the on-demand pricing. + """ + bidadv = AWSBidAdvisor(REFRESH_INTERVAL, REFRESH_INTERVAL, REGION) + assert len(bidadv.on_demand_price_dict) == 0 + updater = bidadv.OnDemandUpdater(bidadv) + updater.get_on_demand_pricing() + assert len(bidadv.on_demand_price_dict) > 0 + + def test_ba_spot_pricing(self): + """ + Tests that the AWSBidVisor correctly gets the spot instance pricing. + """ + bidadv = AWSBidAdvisor(REFRESH_INTERVAL, REFRESH_INTERVAL, REGION) + assert len(bidadv.spot_price_list) == 0 + updater = bidadv.SpotInstancePriceUpdater(bidadv) + updater.get_spot_price_info() + assert len(bidadv.spot_price_list) > 0 + + def test_ba_price_update(self): + """ + Tests that the AXBidVisor actually updates the pricing info. + """ + bidadv = AWSBidAdvisor(REFRESH_INTERVAL, REFRESH_INTERVAL, REGION) + od_updater = bidadv.OnDemandUpdater(bidadv) + od_updater.get_on_demand_pricing() + + sp_updater = bidadv.SpotInstancePriceUpdater(bidadv) + sp_updater.get_spot_price_info() + + # Verify that the pricing info was populated. + assert len(bidadv.on_demand_price_dict) > 0 + assert len(bidadv.spot_price_list) > 0 + + # Make the price dicts empty to check if they get updated. + bidadv.on_demand_price_dict = {} + bidadv.spot_price_list = {} + + od_updater.get_on_demand_pricing() + sp_updater.get_spot_price_info() + + # Verify that the pricing info is populated again. + assert len(bidadv.on_demand_price_dict) > 0 + assert len(bidadv.spot_price_list) > 0 + + def test_ba_get_bid(self): + """ + Tests that the bid_advisor's get_new_bid() method returns correct + bid information. + """ + bidadv = AWSBidAdvisor(REFRESH_INTERVAL, REFRESH_INTERVAL, REGION) + + instance_type = "m3.large" + zone = "us-west-2b" + # Manually populate the prices so that spot-instance prices are chosen. + bidadv.on_demand_price_dict["m3.large"] = "100" + bidadv.spot_price_list = [{'InstanceType': instance_type, + 'SpotPrice': '80', + 'AvailabilityZone': zone}] + bid_info = bidadv.get_new_bid(zone, instance_type) + assert bid_info is not None, "BidAdvisor didn't return any " + \ + "new bid information." + assert bid_info["type"] == "spot" + assert isinstance(bid_info["price"], str) + + # Manually populate the prices so that on-demand instances are chosen. + bidadv.spot_price_list = [{'InstanceType': instance_type, + 'SpotPrice': '85', + 'AvailabilityZone': zone}] + bid_info = bidadv.get_new_bid(zone, instance_type) + assert bid_info is not None, "BidAdvisor didn't return any now " + \ + "bid information." + assert bid_info["type"] == "on-demand" + + def test_ba_get_bid_no_data(self): + """ + Tests that the BidAdvisor returns the default if the pricing + information hasn't be obtained yet. + """ + bidadv = AWSBidAdvisor(REFRESH_INTERVAL, REFRESH_INTERVAL, REGION) + bid_info = bidadv.get_new_bid('us-west-2a', 'm3.large') + assert bid_info["type"] == "on-demand" + + def test_ba_get_current_price(self): + """ + Tests that the BidAdvisor returns the most recent price information. + """ + bidadv = AWSBidAdvisor(REFRESH_INTERVAL, REFRESH_INTERVAL, REGION) + + od_updater = bidadv.OnDemandUpdater(bidadv) + od_updater.get_on_demand_pricing() + + sp_updater = bidadv.SpotInstancePriceUpdater(bidadv) + sp_updater.get_spot_price_info() + + # Verify that the pricing info was populated. + assert len(bidadv.on_demand_price_dict) > 0 + assert len(bidadv.spot_price_list) > 0 + + price_info_map = bidadv.get_current_price() + assert price_info_map["spot"] is not None + assert price_info_map["on-demand"] is not None diff --git a/platform/tests/minion_manager/aws/aws_minion_manager_test.py b/platform/tests/minion_manager/aws/aws_minion_manager_test.py new file mode 100644 index 000000000000..1611402c8fab --- /dev/null +++ b/platform/tests/minion_manager/aws/aws_minion_manager_test.py @@ -0,0 +1,231 @@ +"""The file has unit tests for the AWSMinionManager.""" + +import unittest +import mock +import pytest +from ax.platform.minion_manager.cloud_provider.aws.aws_minion_manager import AWSMinionManager +from ax.platform.minion_manager.cloud_provider.aws.aws_bid_advisor import AWSBidAdvisor +from moto import mock_autoscaling, mock_sts, mock_ec2 +import boto3 +from bunch import bunchify + + +class AWSMinionManagerTest(unittest.TestCase): + """ + Tests for the AWSMinionManager. + """ + cluster_name = "cluster" + cluster_id = "abcd-c0ffeec0ffee" + cluster_name_id = cluster_name + "-" + cluster_id + asg_name = cluster_name_id + "-asg" + lc_name = cluster_name_id + "-lc" + + session = boto3.Session(region_name="us-west-2") + autoscaling = session.client("autoscaling") + + @mock_autoscaling + @mock_sts + def create_mock_asgs(self): + """ + Creates mocked AWS resources. + """ + response = self.autoscaling.create_launch_configuration( + LaunchConfigurationName=self.lc_name, ImageId='ami-f00bad', + SpotPrice="0.100", KeyName='kubernetes-some-key') + resp = bunchify(response) + assert resp.ResponseMetadata.HTTPStatusCode == 200 + + response = self.autoscaling.create_auto_scaling_group( + AutoScalingGroupName=self.asg_name, + LaunchConfigurationName=self.lc_name, MinSize=3, MaxSize=3, + DesiredCapacity=3, + Tags=[{'ResourceId': self.cluster_name_id, + 'Key': 'KubernetesCluster', 'Value': self.cluster_name_id}] + ) + resp = bunchify(response) + assert resp.ResponseMetadata.HTTPStatusCode == 200 + + def basic_setup_and_test(self): + """ + Creates the mock setup for tests, creates the aws_mm object and does + some basic sanity tests before returning it. + """ + self.create_mock_asgs() + aws_mm = AWSMinionManager([self.asg_name], "us-west-2") + assert len(aws_mm.get_asg_metas()) == 0, \ + "ASG Metadata already populated?" + + aws_mm.discover_asgs() + assert aws_mm.get_asg_metas() is not None, "ASG Metadata not populated" + + for asg in aws_mm.get_asg_metas(): + assert asg.asg_info.AutoScalingGroupName == self.asg_name + + aws_mm.populate_current_config() + return aws_mm + + @mock_autoscaling + @mock_sts + def test_discover_asgs(self): + """ + Tests that the discover_asgs method works as expected. + """ + self.basic_setup_and_test() + + @mock_autoscaling + @mock_sts + @mock_ec2 + def test_populate_instances(self): + """ + Tests that existing info. about ASGs is populated correctly. + """ + aws_mm = self.basic_setup_and_test() + asg = aws_mm.get_asg_metas()[0] + + orig_instance_count = len(asg.get_instance_info()) + aws_mm.populate_instances(asg) + assert len(asg.get_instance_info()) == orig_instance_count + 3 + + @mock_autoscaling + @mock_sts + def test_populate_current_config(self): + """ + Tests that existing instances are correctly populated by the + populate_instances() method. + """ + aws_mm = self.basic_setup_and_test() + for asg_meta in aws_mm.get_asg_metas(): + assert asg_meta.get_lc_info().LaunchConfigurationName == \ + self.lc_name + assert asg_meta.get_bid_info()["type"] == "spot" + assert asg_meta.get_bid_info()["price"] == "0.100" + + @mock_autoscaling + @mock_sts + @pytest.mark.skip( + reason="Moto doesn't have some fields in it's LaunchConfig.") + def test_update_cluster_spot(self): + """ + Tests that the AWSMinionManager correctly creates launch-configs and + updates the ASG. + + Note: Moto doesn't have the ClassicLinkVPCSecurityGroups and + IamInstanceProfile fields in it's LaunchConfig. Running the test below + required manually commenting out these fields in the call to + create_launch_configuration :( + """ + awsmm = self.basic_setup_and_test() + bid_info = {} + bid_info["type"] = "spot" + bid_info["price"] = "10" + awsmm.update_scaling_group(awsmm.get_asg_metas()[0], bid_info) + + @mock_autoscaling + @mock_sts + @pytest.mark.skip( + reason="Moto doesn't have some fields in it's LaunchConfig.") + def test_update_cluster_on_demand(self): + """ + Tests that the AWSMinionManager correctly creates launch-configs and + updates the ASG. + + Note: Moto doesn't have the ClassicLinkVPCSecurityGroups and + IamInstanceProfile fields in it's LaunchConfig. Running the test below + required manually commenting out these fields in the call to + create_launch_configuration :( + """ + awsmm = self.basic_setup_and_test() + bid_info = {"type": "on-demand"} + awsmm.update_scaling_group(awsmm.get_asg_metas()[0], bid_info) + + @mock_autoscaling + @mock_sts + @mock_ec2 + def test_update_needed(self): + """ + Tests that the AWSMinionManager correctly checks if updates are needed. + """ + awsmm = self.basic_setup_and_test() + + asg_meta = awsmm.get_asg_metas()[0] + # Moto returns that all instances are running. No updates needed. + assert awsmm.update_needed(asg_meta) is False + bid_info = {"type": "on-demand"} + asg_meta.set_bid_info(bid_info) + + assert awsmm.update_needed(asg_meta) is True + + @mock_autoscaling + @mock_sts + def test_bid_equality(self): + """ + Tests that 2 bids are considered equal when their type and price match. + Not equal otherwise. + """ + a_bid = {} + a_bid["type"] = "on-demand" + b_bid = {} + b_bid["type"] = "on-demand" + b_bid["price"] = "100" + awsmm = self.basic_setup_and_test() + assert awsmm.are_bids_equal(a_bid, b_bid) is True + + # Change type of new bid to "spot". + b_bid["type"] = "spot" + assert awsmm.are_bids_equal(a_bid, b_bid) is False + + # Change the type of a_bid to "spot" but a different price. + a_bid["type"] = "spot" + a_bid["price"] = "90" + assert awsmm.are_bids_equal(a_bid, b_bid) is False + + a_bid["price"] = "100" + assert awsmm.are_bids_equal(a_bid, b_bid) is True + + @mock_autoscaling + @mock_ec2 + @mock_sts + def test_awsmm_instances(self): + """ + Tests that the AWSMinionManager correctly tracks running instances. + """ + awsmm = self.basic_setup_and_test() + asg_meta = awsmm.get_asg_metas()[0] + assert awsmm.check_scaling_group_instances(asg_meta) + + # Update the desired # of instances in the ASG. Verify that + # minion-manager continues to account for the new instances. + self.autoscaling.update_auto_scaling_group( + AutoScalingGroupName=self.asg_name, MaxSize=4, DesiredCapacity=4) + assert awsmm.check_scaling_group_instances(asg_meta) + + @mock_autoscaling + @mock_ec2 + @mock_sts + def test_instance_termination(self): + """ + Tests that the AWSMinionManager schedules instance termination. + """ + awsmm = self.basic_setup_and_test() + assert len(awsmm.on_demand_kill_threads) == 0 + asg_meta = awsmm.get_asg_metas()[0] + awsmm.populate_instances(asg_meta) + + assert len(asg_meta.get_instances()) == 3 + awsmm.schedule_instance_termination(asg_meta) + assert len(awsmm.on_demand_kill_threads) == 3 + + # For testing manually run the run_or_die method. + instance_type = "m3.medium" + zone = "us-west-2b" + awsmm.bid_advisor.on_demand_price_dict[instance_type] = "100" + awsmm.bid_advisor.spot_price_list = [{'InstanceType': instance_type, + 'SpotPrice': '80', + 'AvailabilityZone': zone}] + awsmm.instance_type = instance_type + dict_copy = awsmm.on_demand_kill_threads.copy() + for key in dict_copy.keys(): + awsmm.run_or_die(key, zone, instance_type, asg_meta) + assert len(awsmm.on_demand_kill_threads) == 0 + assert len(asg_meta.get_instances()) == 0 + diff --git a/platform/tests/minion_manager/broker_test.py b/platform/tests/minion_manager/broker_test.py new file mode 100644 index 000000000000..65655cbce583 --- /dev/null +++ b/platform/tests/minion_manager/broker_test.py @@ -0,0 +1,26 @@ +"""The file has unit tests for the cloud broker.""" + +import unittest +import pytest +from ax.platform.minion_manager.cloud_broker import Broker +from ax.platform.minion_manager.cloud_provider.aws.aws_minion_manager import AWSMinionManager + + +class BrokerTest(unittest.TestCase): + """ + Tests for cloud broker. + """ + + def test_get_impl_object(self): + """ + Tests that the get_impl_object method works as expected. + """ + + # Verify that a minion-manager object is returned for "aws" + mgr = Broker.get_impl_object("aws", ["asg_1"], "us-west-2") + assert mgr is not None,"No minion-manager returned!" + assert isinstance(mgr, AWSMinionManager), "Wrong minion-manager instance returned." + + # For non-aws clouds, a NotImplementedError is returned. + with pytest.raises(NotImplementedError): + mgr = Broker.get_impl_object("google", [], "") diff --git a/saas/axops/src/ui/src/app/services/index.ts b/saas/axops/src/ui/src/app/services/index.ts index 1eeb505d2e1a..522842172169 100644 --- a/saas/axops/src/ui/src/app/services/index.ts +++ b/saas/axops/src/ui/src/app/services/index.ts @@ -44,4 +44,3 @@ export { SlackService } from './slack.service'; export * from './playground-info.service'; export * from './system-request.service'; export * from './retention-policy.service'; -export * from './shared.service'; diff --git a/saas/axops/src/ui/src/app/services/services.module.ts b/saas/axops/src/ui/src/app/services/services.module.ts index 33f01a9568f6..abf68c5600b3 100644 --- a/saas/axops/src/ui/src/app/services/services.module.ts +++ b/saas/axops/src/ui/src/app/services/services.module.ts @@ -50,7 +50,6 @@ import { PlaygroundInfoService } from './playground-info.service'; import { TrackingService } from './tracking.service'; import { SlackService } from './slack.service'; import { SystemRequestService } from './system-request.service'; -import { SharedService } from './shared.service'; @NgModule({ providers: [ @@ -98,7 +97,6 @@ import { SharedService } from './shared.service'; GlobalSearchService, SecretService, TrackingService, - SharedService, PlaygroundInfoService, { provide: ContentService, diff --git a/saas/axops/src/ui/src/app/services/shared.service.ts b/saas/axops/src/ui/src/app/services/shared.service.ts deleted file mode 100644 index c3a7390f34db..000000000000 --- a/saas/axops/src/ui/src/app/services/shared.service.ts +++ /dev/null @@ -1,11 +0,0 @@ -import { Injectable } from '@angular/core'; -import {BehaviorSubject} from 'rxjs/BehaviorSubject'; - -@Injectable() -export class SharedService { - - updateSource: BehaviorSubject = new BehaviorSubject({}); - - // tslint:disable-next-line:no-empty - constructor() {} -} diff --git a/saas/axops/src/ui/src/app/views/+user-management/user-profile/user-profile.component.ts b/saas/axops/src/ui/src/app/views/+user-management/user-profile/user-profile.component.ts index 7912c774d430..db73cbe3c913 100644 --- a/saas/axops/src/ui/src/app/views/+user-management/user-profile/user-profile.component.ts +++ b/saas/axops/src/ui/src/app/views/+user-management/user-profile/user-profile.component.ts @@ -3,7 +3,7 @@ import { Router, ActivatedRoute } from '@angular/router'; import { Subscription } from 'rxjs'; import { User } from '../../../model'; -import { UsersService, AuthenticationService, SharedService } from '../../../services'; +import { UsersService, AuthenticationService } from '../../../services'; import { LayoutSettings, HasLayoutSettings } from '../../layout'; import { UserUtils } from '../user-utils'; @@ -27,8 +27,7 @@ export class UserProfileComponent implements OnInit, OnDestroy, LayoutSettings, private usersService: UsersService, private authenticationService: AuthenticationService, private activatedRoute: ActivatedRoute, - private utils: UserUtils, - private sharedService: SharedService) { + private utils: UserUtils) { } public get layoutSettings(): LayoutSettings { @@ -84,15 +83,11 @@ export class UserProfileComponent implements OnInit, OnDestroy, LayoutSettings, } private async loadUser() { - let emitToLayout = !this.user.id; await this.usersService.getUser(this.username).subscribe(result => { this.user = result; if (this.username === this.authenticationService.getUsername()) { this.isCurrentLoggedInUser = true; } - if (emitToLayout) { - this.sharedService.updateSource.next(this.layoutSettings); - } }); } } diff --git a/saas/axops/src/ui/src/app/views/layout/layout.component.ts b/saas/axops/src/ui/src/app/views/layout/layout.component.ts index 8393ee51535a..bd02859df638 100644 --- a/saas/axops/src/ui/src/app/views/layout/layout.component.ts +++ b/saas/axops/src/ui/src/app/views/layout/layout.component.ts @@ -1,6 +1,6 @@ import * as moment from 'moment'; import { Component, ViewChild, OnInit, OnDestroy } from '@angular/core'; -import { RouterOutlet, Router, NavigationEnd, ActivatedRoute } from '@angular/router'; +import { RouterOutlet, Router, ActivatedRoute } from '@angular/router'; import { Subject, Subscription, Observable } from 'rxjs'; import { FormControl, FormGroup, Validators } from '@angular/forms'; @@ -15,7 +15,6 @@ import { SecretService, ViewPreferencesService, AuthenticationService, - SharedService } from '../../services'; import { Task, Application } from '../../model'; @@ -70,7 +69,6 @@ export class LayoutComponent implements OnInit, OnDestroy { public jiraIssueCreatorPanelComponent: JiraIssueCreatorPanelComponent; @ViewChild(JiraIssuesPanelComponent) public jiraIssuesPanelComponent: JiraIssuesPanelComponent; - public layoutSettings: LayoutSettings = {}; public globalSearch: GlobalSearchSetting; public hiddenScrollbar: boolean; public openedPanelOffCanvas: boolean; @@ -101,21 +99,13 @@ export class LayoutComponent implements OnInit, OnDestroy { private playgroundInfoService: PlaygroundInfoService, private notificationService: NotificationService, private viewPreferencesService: ViewPreferencesService, - private authenticationService: AuthenticationService, - private sharedService: SharedService) { + private authenticationService: AuthenticationService) { this.encryptForm = new FormGroup({ repo: new FormControl('', Validators.required), secret: new FormControl('', Validators.required), }); - this.subscriptions.push(router.events.subscribe(event => { - if (event instanceof NavigationEnd) { - let component: any = this.routerOutlet.component; - this.layoutSettings = component ? component.layoutSettings || {} : {}; - } - })); - this.subscriptions.push(this.slidingPanelService.panelOpened.subscribe( isHidden => setTimeout(() => this.hiddenScrollbar = isHidden))); @@ -144,10 +134,6 @@ export class LayoutComponent implements OnInit, OnDestroy { this.playgroundTask = info; })); - this.subscriptions.push(this.sharedService.updateSource.subscribe((layout) => { - this.layoutSettings = layout; - })); - this.authenticationService.getCurrentUser().then(user => { this.subscriptions.push(Observable.merge( this.notificationService.getEventsStream(user.username), @@ -191,6 +177,11 @@ export class LayoutComponent implements OnInit, OnDestroy { })); } + public get layoutSettings(): LayoutSettings { + let component: any = this.routerOutlet.isActivated ? this.routerOutlet.component : null; + return component ? component.layoutSettings || {} : {}; + } + public ngOnInit() { this.launchPanelService.initPanel(this.multipleServiceLaunchPanel); }