Skip to content
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

compute: implement autoscaler #1137

Merged

Conversation

stephenplusplus
Copy link
Contributor

For #1073

Fixes #1152

Implements Autoscaler support.

To Dos

  • Docs
  • Tests
    • System
      • Figure out how to create an instance group before the tests run
    • Unit
      • autoscaler.js
      • index.js
      • zone.js

Creating an autoscaler...

// Easy way
var config = {
  coolDown: 30,
  cpu: 80,
  loadBalance: 40,
  maxReplicas: 5,
  minReplicas: 0,
  target: 'instance-group-name'
};

// Hard (but supported) way
var config = {
  autoscalingPolicy: {
    coolDownPeriodSec: 30,
    cpuUtilization: {
      utilizationTarget: 0.8
    },
    loadBalancingUtilization: {
      utilizationTarget: 0.4
    },
    maxNumReplicas: 5,
    minNumReplicas: 0
  },
  target: 'https://content.googleapis.com/compute/v1/projects/{PROJECT_ID}/zones/{ZONE_ID}/instanceGroupMangers/instance-group-name'
};

var zone = compute.zone('us-central1-a');
zone.createAutoscaler('autoscaler-namer', config, function(err, operation, autoscaler, apiResponse) {
  // ...
});

@stephenplusplus stephenplusplus added the api: compute Issues related to the Compute Engine API. label Feb 26, 2016
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 26, 2016
@callmehiphop callmehiphop mentioned this pull request Feb 26, 2016
20 tasks
@stephenplusplus stephenplusplus force-pushed the spp--1073-autoscaler branch 2 times, most recently from 5efac05 to 12d00b9 Compare February 26, 2016 19:10
@@ -439,6 +439,126 @@ Compute.prototype.getAddresses = function(options, callback) {
};

/**
* Get a list of autoscalers. For a detailed description of this method's
* options, see [API reference](https://cloud.google.com/compute/docs/reference/v1/autoscalers/aggregatedList).

This comment was marked as spam.

This comment was marked as spam.

@stephenplusplus
Copy link
Contributor Author

Fixed those things!

@callmehiphop
Copy link
Contributor

Locally I'm getting an error when testing the docs on v0.12.7.

Uncaught TypeError: The callback's first argument must be a status code

When running against v5.6.0 npm run test seems to be timing out on something. I think this might relate to gRPC, but TBH I didn't look into it very much.

Other than those errors and a few documentation nitpicks, this LGTM!

@stephenplusplus
Copy link
Contributor Author

My gosh, gRPC is haunting us. I'll try to get that error on 0.12.7.

@stephenplusplus
Copy link
Contributor Author

If you strictly run node_modules/mocha/bin/mocha test/docs.js, you get the Uncaught TypeError?

@callmehiphop
Copy link
Contributor

If you strictly run node_modules/mocha/bin/mocha test/docs.js, you get the Uncaught TypeError?

Nope!

@callmehiphop
Copy link
Contributor

Although it is still hanging on something

@stephenplusplus
Copy link
Contributor Author

Please check all that apply:

v0.12.7

  • npm test hangs
  • node_modules/mocha/bin/mocha test/docs.js hangs
  • npm test returns Uncaught TypeError: The callback's first argument must be a status code
  • node_modules/mocha/bin/mocha test/docs.js returns Uncaught TypeError: The callback's first argument must be a status code

v5.6.0

  • npm test hangs
  • node_modules/mocha/bin/mocha test/docs.js hangs
  • npm test returns Uncaught TypeError: The callback's first argument must be a status code
  • node_modules/mocha/bin/mocha test/docs.js returns Uncaught TypeError: The callback's first argument must be a status code

As you probably guessed, none of the above happens for me locally. There's obviously something going on, but I don't really know where to start to find it.

@stephenplusplus
Copy link
Contributor Author

@stephenplusplus
Copy link
Contributor Author

If you run the same tests prior to this commit, will you get the same errors?

@stephenplusplus
Copy link
Contributor Author

I pushed a commit to see if anything improves. Travis is never happy, so I'm not expecting it to win him over. But maybe it'll help what you're getting locally. If it does, I'll send a different PR with that change.

@callmehiphop
Copy link
Contributor

If you run the same tests prior to this commit, will you get the same errors?

I don't, I'm somehow magically all green when I run against master.

@stephenplusplus
Copy link
Contributor Author

It makes no sense, man!!

@callmehiphop
Copy link
Contributor

I just pulled your latest commit! It helped, lol.

@stephenplusplus
Copy link
Contributor Author

That's awesome. The only thing I can't line up is why this PR revealed the problem. Oh well, as long as it works for us all, that's all that matters. I'll just add a test for it and we can merge it in with this PR. We talked about it most here anyway, so finding it later, this PR would help.

I'm thinking it solves #1140 and #1152 - wdyt?

@callmehiphop
Copy link
Contributor

SGTM.. I'm a little lost on what's going on, but maybe we should open a new issue for investigating gRPC stuff since it has been giving us weird/random errors.

@stephenplusplus
Copy link
Contributor Author

Yeah, we can leave #1140 open as a place to log issues that might relate to gRPC.

I think this is ready for a merge!

callmehiphop added a commit that referenced this pull request Mar 7, 2016
@callmehiphop callmehiphop merged commit 71711bd into googleapis:master Mar 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: compute Issues related to the Compute Engine API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants