Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 1
Allow charon to search aliases when fetching deps#1898
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
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
eb4b2a1cdfaea45a337a804e7385fb8d03b169df506d02e698123495b2ea7cb785f8d10612cdf520ebf4f893195cfcb1be7c8d013d967846File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1158,37 +1158,92 @@ InstanceSchema.methods.getDependencies = function (params, cb) { | ||
| cb = params | ||
| params = {} | ||
| } | ||
| var self = this | ||
| return Instance.findByIdAsync(this._id) | ||
| .tap(function (instance) { | ||
| .tap(instance => { | ||
| if (!instance) { | ||
| // the update failed | ||
| throw Boom.notFound('This instance could not be found!', { | ||
| instance: self._id.toString() | ||
| throw new Instance.NotFoundError('This instance could not be found!', { | ||
| instance: this._id.toString() | ||
| }) | ||
| } | ||
| }) | ||
| .then(function (instance) { | ||
| return instance.dependencies || [] | ||
| }) | ||
| .filter(function removeSelf (dep) { | ||
| return self._id.toString() !== dep.id | ||
| }) | ||
| .filter(function filterByHostname (dep) { | ||
| if (!params.hostname) { | ||
| return true | ||
| } | ||
| return params.hostname === dep.elasticHostname | ||
| .then(instance => { | ||
| // this method will throw if there isn't a hostname, or an alias. So use that to control | ||
| // the flow of this | ||
| return instance.convertAliasToDependency(params.hostname) | ||
| .then(dependency => [dependency]) | ||
| .catch(Instance.NotFoundError, Instance.IncorrectStateError, () => { | ||
| // It wasn't an alias, so maybe it's a dep? | ||
| let dependencies = instance.dependencies || [] | ||
| if (params.hostname) { | ||
| // Only get the dependency that matches the hostname | ||
| dependencies = dependencies.filter(dep => params.hostname === dep.elasticHostname) | ||
| } else { | ||
| // Remove self from the list (if it exists) | ||
| dependencies = dependencies.filter(dep => this._id.toString() !== dep.id) | ||
| } | ||
| // Annotate dependencies with additional instance information (currently | ||
| // only adding network information for charon) | ||
| return Promise | ||
| .map(dependencies, dep => Instance.findByIdAsync(dep.instanceId)) | ||
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we batch this into one fetch? This is going to be called a bunch from charon I'm guessing and speed of this is important. Unless we don't think it's a concern for the moment. MemberAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. charon only calls it with the hostname, the UI calls for all of them, so it should only ever be 1 call. Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see. So it's the UI call that'll be slow? Otherwise what's up with the else on line 1182, and what's the scenario it'd be ran and how that'd only be 1 array item. MemberAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It was when instances had deps of themselves. This was a to remove them from showing up in the UI. I'm not sure what you mean about the 1 item array | ||
| .filter(exists) | ||
| }) | ||
| }) | ||
| .map(function fetchInstance (dep) { | ||
| // Annotate dependencies with additional instance information (currently | ||
| // only adding network information for charon) | ||
| return Instance.findByIdAsync(dep.instanceId) | ||
| .catch(Instance.NotFoundError, err => { | ||
| // the update failed | ||
| throw Boom.notFound('This instance could not be found!', { err }) | ||
| }) | ||
| .filter(exists) | ||
| .asCallback(cb) | ||
| } | ||
| /** | ||
| * Given an alias (hostname), find the instance being referred to, and resolve it | ||
| * | ||
| * @param {String} alias - Hostname that some instance is using to reference another instance | ||
| * | ||
| * @resolves {Instance} - Dependent Instance model referenced by the given alias | ||
| * | ||
| * @throws Instance.IncorrectStateError - When we're looking for a dependency in an instance | ||
| * isn't a masterpod nor isolated | ||
| * @throws Instance.NotFoundError - When no alias is given | ||
| * @throws Instance.NotFoundError - When the alias isn't present in this Instance | ||
| * @throws Instance.NotFoundError - When the Dependency Instance fetch fails | ||
| * | ||
| */ | ||
| InstanceSchema.methods.convertAliasToDependency = Promise.method(function (alias) { | ||
| var log = logger.log.child({ | ||
| instanceId: keypather.get(this, '_id'), | ||
| aliases: keypather.get(this, 'aliases'), | ||
| alias, | ||
| method: 'InstanceSchema.methods.convertAliasesToDeps' | ||
| }) | ||
| log.info('called') | ||
| if (!alias) { | ||
| throw new Instance.NotFoundError({ alias }) | ||
| } | ||
| const base64Alias = new Buffer(alias).toString('base64') | ||
| const aliasModel = this.aliases[base64Alias] | ||
| if (!aliasModel) { | ||
| throw new Instance.NotFoundError({ alias, base64Alias, aliases: this.aliases }) | ||
| } | ||
| const query = { | ||
| 'contextVersion.context': aliasModel.contextId | ||
| } | ||
| if (this.masterPod) { | ||
| query.masterPod = true | ||
| } else if (this.isolated) { | ||
| query.isolated = this.isolated | ||
| } else { | ||
| // This shouldn't happen, so if it does, alert Nathan | ||
| throw new Instance.IncorrectStateError('be masterPod or isolated', 'neither') | ||
| } | ||
| return Instance.findOneAsync(query) | ||
| .tap(instance => { | ||
| if (!instance) { | ||
| throw new Instance.NotFoundError(query) | ||
| } | ||
| }) | ||
| }) | ||
| /** | ||
| * Fetch all of the MasterPods that should be autoForked, given the list of instances which should | ||
| * be autoDeployed. By using this list, we can find all of the child instances that were updated. | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we not add
.bind(this)here and not use self?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done