(18.37.0) DEVOPS-11415: adds asg_logical_name arg #44

taoistmath · 2018-07-31T21:29:05Z

https://dunandb.jira.com/browse/DEVOPS-11415

Tested by updating salt state for pip installing License2Deploy to use my branch and highstated a packer agent with these changes:

[root@ae1-saltmaster43-ci slave (release-next *)]# pwd
/srv/config/ciq/salt/ci/jenkins/slave
[root@ae1-saltmaster43-ci slave (release-next *)]# cat init.sls | grep -A 7 'License2Deploy:'
License2Deploy:
  pip.installed:
    - bin_env: /usr/bin/pip2.7
    - name: git+https://github.com/taoistmath/License2Deploy.git@DEVOPS-11415
    - upgrade: True
    - require:
      - file: {{ opt }}/License2Deploy/regions.yml
      - pkg: python27

[INFO    ] Executing state pip.installed for git+https://github.com/taoistmath/License2Deploy.git@DEVOPS-11415
[DEBUG   ] Installed pip version: 7.1.0
[INFO    ] Executing command ['/usr/bin/pip2.7', 'install', '--upgrade', 'git+https://github.com/taoistmath/License2Deploy.git@DEVOPS-11415'] in directory '/root'
[DEBUG   ] stdout: Downloading/unpacking git+https://github.com/taoistmath/License2Deploy.git@DEVOPS-11415
  Cloning https://github.com/taoistmath/License2Deploy.git (to DEVOPS-11415) to /tmp/pip-hwT2R2-build
  Running setup.py (path:/tmp/pip-hwT2R2-build/setup.py) egg_info for package from git+https://github.com/taoistmath/License2Deploy.git@DEVOPS-11415
    
Requirement already up-to-date: boto==2.48.0 in /usr/lib/python2.7/site-packages (from License2Deploy==0.2.0)
Requirement already up-to-date: boto3==1.7.37 in /usr/lib/python2.7/site-packages (from License2Deploy==0.2.0)
Requirement already up-to-date: PyYAML==3.12 in /usr/lib/python2.7/site-packages (from License2Deploy==0.2.0)
Requirement already up-to-date: retry==0.9.2 in /usr/lib/python2.7/site-packages (from License2Deploy==0.2.0)
Installing collected packages: License2Deploy
  Found existing installation: License2Deploy 0.2.0
    Uninstalling License2Deploy:
      Successfully uninstalled License2Deploy
  Running setup.py install for License2Deploy
    
    Installing rolling_deploy script to /usr/bin
Successfully installed License2Deploy
Cleaning up...
[INFO    ] {'git+https://github.com/taoistmath/License2Deploy.git@DEVOPS-11415==???': 'Installed'}
[INFO    ] Completed state [git+https://github.com/taoistmath/License2Deploy.git@DEVOPS-11415] at time 09:08:44.526112
[INFO    ] Running state [/etc/salt/master] at time 09:08:44.526537

I updated cos-service_qa_deploy_ami to use the new flag:

Ran the job and saw that instances were launched in the proper ASG, as evidenced by not having Public IPs and today as the launch date:

coveralls · 2018-07-31T21:32:08Z

Coverage increased (+0.02%) to 97.0% when pulling 336c764 on taoistmath:DEVOPS-11415 into b1c5d95 on dandb:master.

gh4x · 2018-08-01T00:23:08Z

License2Deploy/rolling_deploy.py

@@ -97,8 +99,9 @@ def get_autoscale_group_name(self):

  def get_autoscaling_group_name_from_cloudformation(self):
    if not self.autoscaling_group:


indent the above more?

I actually see a few two space indents in functions across this file.

I have set my spacing to 2 spaces in accordance with the rest of the file.

gh4x · 2018-08-01T00:25:45Z

License2Deploy/rolling_deploy.py

@@ -97,8 +99,9 @@ def get_autoscale_group_name(self):

  def get_autoscaling_group_name_from_cloudformation(self):
    if not self.autoscaling_group:


I actually see a few two space indents in functions across this file.

gh4x · 2018-08-01T00:54:59Z

License2Deploy/rolling_deploy.py

@@ -400,13 +403,16 @@ def get_args(): # pragma: no cover
  parser.add_argument('-r', '--ready-wait', action='store', dest='ready_wait', help='Wait time for ec2 instance to be ready', type=int, nargs=2, default=[10, 30])
  parser.add_argument('-H', '--health-wait', action='store', dest='health_wait', help='Wait time for ec2 instance health check', type=int, nargs=2, default=[10, 30])
  parser.add_argument('-o', '--only-new-wait', action='store', dest='only_new_wait', help='Wait time for old ec2 instances to terminate', type=int, nargs=2, default=[10, 30])
+  parser.add_argument('-A', '--asg-logical-name', action='store', dest='asg_logical_name', help='ASG Logical Name from CFN', type=str)


hmm. I don't like these 1-letter arguments. especially with -a already being taken by --ami.

As I understand it, these are not used. I would say use a different letter at least if you want to keep in the 1-letter args.

will approve if you think there is a good reason to leave as is

These are valid concerns. Single character arguments are pretty standard. Not sure what you mean by 'not used', these are definitely used, the command from deploy_ami job is /usr/bin/rolling_deploy -f ${FORCE_REDEPLOY} ${DEPLOY_OPTIONS} -e qa -p cos-service -b ${BUILD_NUM} -P dnbi-deploy -a ${AMI} -s ${stack_name}.

Do you have a recommendation for what to use instead of -A? I ask b/c we use -c/-C and -p/-P already in this script and I chose -A since ASG is capitalized in the logical name so it made sense. I don't mind changing if you have a suggestion for something that would make more sense.

Oh wow. I assumed the deploy_ami job would have used the longer args so you could understand the difference between -p/-P. Especially since they are in a jenkins job and not something that is being typed repeatedly. I vote for updating that job to use longer args but will approve this now as it is keeping with the existing convention.

gh4x · 2018-08-01T15:51:08Z

License2Deploy/rolling_deploy.py

@@ -400,13 +403,16 @@ def get_args(): # pragma: no cover
  parser.add_argument('-r', '--ready-wait', action='store', dest='ready_wait', help='Wait time for ec2 instance to be ready', type=int, nargs=2, default=[10, 30])
  parser.add_argument('-H', '--health-wait', action='store', dest='health_wait', help='Wait time for ec2 instance health check', type=int, nargs=2, default=[10, 30])
  parser.add_argument('-o', '--only-new-wait', action='store', dest='only_new_wait', help='Wait time for old ec2 instances to terminate', type=int, nargs=2, default=[10, 30])
+  parser.add_argument('-A', '--asg-logical-name', action='store', dest='asg_logical_name', help='ASG Logical Name from CFN', type=str)


Oh wow. I assumed the deploy_ami job would have used the longer args so you could understand the difference between -p/-P. Especially since they are in a jenkins job and not something that is being typed repeatedly. I vote for updating that job to use longer args but will approve this now as it is keeping with the existing convention.

mprince approved these changes Jul 31, 2018

View reviewed changes

gh4x reviewed Aug 1, 2018

View reviewed changes

gh4x suggested changes Aug 1, 2018

View reviewed changes

DEVOPS-11415: adds asg_logical_name arg

336c764

taoistmath force-pushed the DEVOPS-11415 branch from 6465e71 to 336c764 Compare August 1, 2018 14:39

gh4x approved these changes Aug 1, 2018

View reviewed changes

taoistmath merged commit 37cdc25 into dandb:master Aug 2, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(18.37.0) DEVOPS-11415: adds asg_logical_name arg #44

(18.37.0) DEVOPS-11415: adds asg_logical_name arg #44

taoistmath commented Jul 31, 2018 •

edited

Loading

coveralls commented Jul 31, 2018 •

edited

Loading

gh4x Aug 1, 2018

gh4x Aug 1, 2018 •

edited

Loading

taoistmath Aug 1, 2018

gh4x Aug 1, 2018 •

edited

Loading

gh4x Aug 1, 2018

gh4x Aug 1, 2018

taoistmath Aug 1, 2018

gh4x Aug 1, 2018

gh4x Aug 1, 2018

		@@ -97,8 +99,9 @@ def get_autoscale_group_name(self):

		def get_autoscaling_group_name_from_cloudformation(self):
		if not self.autoscaling_group:

(18.37.0) DEVOPS-11415: adds asg_logical_name arg #44

(18.37.0) DEVOPS-11415: adds asg_logical_name arg #44

Conversation

taoistmath commented Jul 31, 2018 • edited Loading

coveralls commented Jul 31, 2018 • edited Loading

gh4x Aug 1, 2018

Choose a reason for hiding this comment

gh4x Aug 1, 2018 • edited Loading

Choose a reason for hiding this comment

taoistmath Aug 1, 2018

Choose a reason for hiding this comment

gh4x Aug 1, 2018 • edited Loading

Choose a reason for hiding this comment

gh4x Aug 1, 2018

Choose a reason for hiding this comment

gh4x Aug 1, 2018

Choose a reason for hiding this comment

taoistmath Aug 1, 2018

Choose a reason for hiding this comment

gh4x Aug 1, 2018

Choose a reason for hiding this comment

gh4x Aug 1, 2018

Choose a reason for hiding this comment

taoistmath commented Jul 31, 2018 •

edited

Loading

coveralls commented Jul 31, 2018 •

edited

Loading

gh4x Aug 1, 2018 •

edited

Loading

gh4x Aug 1, 2018 •

edited

Loading