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

[ui] Show ALL regions' leaders when viewing servers route #24723

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/24723.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
ui: add leadership status for servers in other regions
```
13 changes: 8 additions & 5 deletions ui/app/models/agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ import { inject as service } from '@ember/service';
import { computed } from '@ember/object';
import Model from '@ember-data/model';
import { attr } from '@ember-data/model';
import classic from 'ember-classic-decorator';
import { tracked } from '@glimmer/tracking';
import { action } from '@ember/object';
import formatHost from 'nomad-ui/utils/format-host';

@classic
export default class Agent extends Model {
@service system;

Expand All @@ -29,9 +29,12 @@ export default class Agent extends Model {
return formatHost(address, rpcPort);
}

@computed('rpcAddr', 'system.leader.rpcAddr')
get isLeader() {
return this.get('system.leader.rpcAddr') === this.rpcAddr;
@tracked isLeader = false;

@action async checkForLeadership() {
const leaders = await this.system.leaders;
this.isLeader = leaders.includes(this.rpcAddr);
return this.isLeader;
Comment on lines +32 to +37
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using tracked is more idiomatic than the isLeader getter in Ember Octane. And since service getters (like this.get('system.leader....) was the thing holding it back as a classic model, removed the @classic tag as well.

}

@computed('tags.build')
Expand Down
5 changes: 0 additions & 5 deletions ui/app/routes/clients.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,6 @@ import classic from 'ember-classic-decorator';
@classic
export default class ClientsRoute extends Route.extend(WithForbiddenState) {
@service store;
@service system;

beforeModel() {
return this.get('system.leader');
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As best as I can tell, this was accidentally left in here by copying the servers route page ~9 years ago.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nomad-bushes


model() {
return RSVP.hash({
Expand Down
10 changes: 6 additions & 4 deletions ui/app/routes/servers.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,16 @@ export default class ServersRoute extends Route.extend(WithForbiddenState) {
@service store;
@service system;

beforeModel() {
return this.get('system.leader');
async beforeModel() {
await this.system.leaders;
}

model() {
async model() {
const agents = await this.store.findAll('agent');
await Promise.all(agents.map((agent) => agent.checkForLeadership()));
return RSVP.hash({
nodes: this.store.findAll('node'),
agents: this.store.findAll('agent'),
agents,
}).catch(notifyForbidden(this));
}
}
28 changes: 12 additions & 16 deletions ui/app/services/system.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,27 +12,23 @@ import { namespace } from '../adapters/application';
import jsonWithDefault from '../utils/json-with-default';
import classic from 'ember-classic-decorator';
import { task } from 'ember-concurrency';

@classic
export default class SystemService extends Service {
@service token;
@service store;

@computed('activeRegion')
get leader() {
const token = this.token;

return PromiseObject.create({
promise: token
.authorizedRequest(`/${namespace}/status/leader`)
.then((res) => res.json())
.then((rpcAddr) => ({ rpcAddr }))
.then((leader) => {
// Dirty self so leader can be used as a dependent key
this.notifyPropertyChange('leader.rpcAddr');
return leader;
}),
});
/**
* Iterates over all regions and returns a list of leaders' rpcAddrs
*/
@computed('regions.[]')
get leaders() {
return Promise.all(
this.regions.map((region) => {
return this.token
.authorizedRequest(`/${namespace}/status/leader?region=${region}`)
.then((res) => res.json());
})
);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Best as I can tell, the only thing that uses system.leader was the servers page, which is now using .leaders instead. Removed the old one.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the ${namespace} referring to here? v1?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, v1! It's my third favourite among the three ways we use the term "namespace" in the UI.

It imports from

export const namespace = 'v1';

}

@computed
Expand Down
23 changes: 15 additions & 8 deletions ui/app/templates/components/server-agent-row.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
~}}

<td data-test-server-name
{{keyboard-shortcut
{{keyboard-shortcut
enumerated=true
action=(action this.goToAgent)
}}
Expand All @@ -17,13 +17,20 @@
/>
</span></td>
<td data-test-server-is-leader>

<Hds::Badge
@text={{if this.agent.isLeader "True" "False"}}
@icon={{if this.agent.isLeader "check-circle" ""}}
@color={{if this.agent.isLeader "success" "neutral"}}
@size="large"
/>
<Hds::Badge
@text={{if
this.agent.isLeader
(if
this.agent.system.shouldShowRegions
(concat "True" " (" this.agent.region ")")
"True"
)
"False"
}}
Comment on lines +21 to +29
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Funky formatting, but:

  1. If not a leader, just show "False"
  2. If a leader, show "True"
  3. If there are multiple regions at play, show region name in parentheses.

@icon={{if this.agent.isLeader "check-circle" ""}}
@color={{if this.agent.isLeader "success" "neutral"}}
@size="large"
/>
</td>
<td data-test-server-address class="is-200px is-truncatable">{{this.agent.address}}</td>
<td data-test-server-port>{{this.agent.serfPort}}</td>
Expand Down
Loading