zuul/zuul
Code Issues Proposed changes

Add options to manage nodes/requests

Browse Source 
This adds an action menu to the nodes and nodeset-requests web UI,
so that admins may set nodes to HOLD or USED through the web UI,
or directly delete nodeset requests.

This provides some ability to intervene in the case of errors or
exceptional situations.

Change-Id: Iceb712e3f42d5b565c135f6a0138181120145958
This commit is contained in:
James E. Blair
2025-05-29 14:16:14 -07:00
parent 6d0451b77c
commit 5d64f52e27
6 changed files with 252 additions and 69 deletions
Download Patch File Download Diff File Expand all files Collapse all files

150
tests/unit/test_web.py
View File

@ -25,6 +25,7 @@ import subprocess
import threading
from unittest import skip, mock
from kazoo.exceptions import NoNodeError
import requests
from zuul.lib.statsd import normalize_statsd_name
@ -91,6 +92,10 @@ class WebMixin:
return requests.post(
urllib.parse.urljoin(self.base_url, url), *args, **kwargs)
def put_url(self, url, *args, **kwargs):
return requests.put(
urllib.parse.urljoin(self.base_url, url), *args, **kwargs)
def delete_url(self, url, *args, **kwargs):
return requests.delete(
urllib.parse.urljoin(self.base_url, url), *args, **kwargs)
@ -1495,6 +1500,20 @@ class TestWebProviders(LauncherBaseTestCase, WebMixin):
config_file = 'zuul-connections-nodepool.conf'
tenant_config_file = 'config/multi-tenant-provider/main.yaml'
def _getToken(self, admin=None):
if admin is None:
admin = ['tenant-one']
authz = {'iss': 'zuul_operator',
'aud': 'zuul.example.com',
'sub': 'testuser',
'zuul': {
'admin': admin,
},
'exp': int(time.time()) + 3600}
token = jwt.encode(authz, key='NoDanaOnlyZuul',
algorithm='HS256')
return token
@simple_layout('layouts/nodepool.yaml', enable_nodepool=True)
def test_web_providers(self):
self.waitUntilSettled()
@ -1626,29 +1645,13 @@ class TestWebProviders(LauncherBaseTestCase, WebMixin):
)
self.assertEqual(401, resp.status_code, resp.text)
# Test that the wrong tenant fails, even with auth
authz = {'iss': 'zuul_operator',
'aud': 'zuul.example.com',
'sub': 'testuser',
'zuul': {
'admin': ['tenant-two', ]
},
'exp': int(time.time()) + 3600}
token = jwt.encode(authz, key='NoDanaOnlyZuul',
algorithm='HS256')
token = self._getToken(['tenant-two'])
resp = self.delete_url(
f"api/tenant/tenant-two/image-build-artifact/{art['uuid']}",
headers={'Authorization': 'Bearer %s' % token})
self.assertEqual(404, resp.status_code, resp.text)
# Do it again with auth
authz = {'iss': 'zuul_operator',
'aud': 'zuul.example.com',
'sub': 'testuser',
'zuul': {
'admin': ['tenant-one', ]
},
'exp': int(time.time()) + 3600}
token = jwt.encode(authz, key='NoDanaOnlyZuul',
algorithm='HS256')
token = self._getToken(['tenant-one'])
resp = self.delete_url(
f"api/tenant/tenant-one/image-build-artifact/{art['uuid']}",
headers={'Authorization': 'Bearer %s' % token})
@ -1694,29 +1697,13 @@ class TestWebProviders(LauncherBaseTestCase, WebMixin):
)
self.assertEqual(401, resp.status_code, resp.text)
# Test that the wrong tenant fails, even with auth
authz = {'iss': 'zuul_operator',
'aud': 'zuul.example.com',
'sub': 'testuser',
'zuul': {
'admin': ['tenant-two', ]
},
'exp': int(time.time()) + 3600}
token = jwt.encode(authz, key='NoDanaOnlyZuul',
algorithm='HS256')
token = self._getToken(['tenant-two'])
resp = self.delete_url(
f"api/tenant/tenant-two/image-upload/{upload['uuid']}",
headers={'Authorization': 'Bearer %s' % token})
self.assertEqual(404, resp.status_code, resp.text)
# Do it again with auth
authz = {'iss': 'zuul_operator',
'aud': 'zuul.example.com',
'sub': 'testuser',
'zuul': {
'admin': ['tenant-one', ]
},
'exp': int(time.time()) + 3600}
token = jwt.encode(authz, key='NoDanaOnlyZuul',
algorithm='HS256')
token = self._getToken(['tenant-one'])
resp = self.delete_url(
f"api/tenant/tenant-one/image-upload/{upload['uuid']}",
headers={'Authorization': 'Bearer %s' % token})
@ -1762,15 +1749,7 @@ class TestWebProviders(LauncherBaseTestCase, WebMixin):
)
self.assertEqual(401, resp.status_code, resp.text)
# Do it again with auth
authz = {'iss': 'zuul_operator',
'aud': 'zuul.example.com',
'sub': 'testuser',
'zuul': {
'admin': ['tenant-one', ]
},
'exp': int(time.time()) + 3600}
token = jwt.encode(authz, key='NoDanaOnlyZuul',
algorithm='HS256')
token = self._getToken(['tenant-one'])
resp = self.post_url(
"api/tenant/tenant-one/image/ubuntu-local/build",
headers={'Authorization': 'Bearer %s' % token})
@ -1782,15 +1761,7 @@ class TestWebProviders(LauncherBaseTestCase, WebMixin):
dict(name='build-ubuntu-local-image', result='SUCCESS'),
], ordered=False)
# Try again with the wrong tenant
authz = {'iss': 'zuul_operator',
'aud': 'zuul.example.com',
'sub': 'testuser',
'zuul': {
'admin': ['tenant-two', ]
},
'exp': int(time.time()) + 3600}
token = jwt.encode(authz, key='NoDanaOnlyZuul',
algorithm='HS256')
token = self._getToken(['tenant-two'])
resp = self.post_url(
"api/tenant/tenant-two/image/ubuntu-local/build",
headers={'Authorization': 'Bearer %s' % token})
@ -1826,15 +1797,7 @@ class TestWebProviders(LauncherBaseTestCase, WebMixin):
], ordered=False)
self.executor_server.hold_jobs_in_build = True
authz = {'iss': 'zuul_operator',
'aud': 'zuul.example.com',
'sub': 'testuser',
'zuul': {
'admin': ['tenant-one', ]
},
'exp': int(time.time()) + 3600}
token = jwt.encode(authz, key='NoDanaOnlyZuul',
algorithm='HS256')
token = self._getToken(['tenant-one'])
resp = self.post_url(
"api/tenant/tenant-one/image/ubuntu-local/build",
headers={'Authorization': 'Bearer %s' % token})
@ -1927,6 +1890,67 @@ class TestWebProviders(LauncherBaseTestCase, WebMixin):
self.executor_server.release()
self.waitUntilSettled()
@simple_layout('layouts/nodepool.yaml', enable_nodepool=True)
def test_web_nodes_hold_delete(self):
# This tests 3 things (since they are lifecycle related):
# * Setting the node state to hold
# * Setting the node state to delete
# * Deleting the request
self.waitUntilSettled()
self.startWebServer()
request = self.requestNodes(['debian-normal'])
self.assertEqual(request.state,
zuul.model.NodesetRequest.State.FULFILLED)
self.assertEqual(len(request.nodes), 1)
nodes = self.get_url('api/tenant/tenant-one/nodes').json()
self.assertEqual(len(nodes), 1)
token = self._getToken(['tenant-one'])
resp = self.put_url(
f"api/tenant/tenant-one/nodes/{nodes[0]['uuid']}",
headers={'Authorization': 'Bearer %s' % token},
json={'state': 'hold'},
)
self.assertEqual(201, resp.status_code)
self.waitUntilSettled()
node = self.launcher.api.nodes_cache.getItem(nodes[0]['uuid'])
self.assertEqual(node.State.HOLD, node.state)
resp = self.put_url(
f"api/tenant/tenant-one/nodes/{nodes[0]['uuid']}",
headers={'Authorization': 'Bearer %s' % token},
json={'state': 'used'},
)
self.assertEqual(201, resp.status_code)
self.waitUntilSettled()
requests = self.get_url(
'api/tenant/tenant-one/nodeset-requests').json()
self.assertEqual(len(requests), 1)
self.assertEqual(request.uuid, requests[0]['uuid'])
resp = self.delete_url(
f"api/tenant/tenant-one/nodeset-requests/{requests[0]['uuid']}",
headers={'Authorization': 'Bearer %s' % token},
)
self.assertEqual(204, resp.status_code)
self.waitUntilSettled()
ctx = self.createZKContext(None)
for _ in iterate_timeout(10, "request to be deleted"):
try:
request.refresh(ctx)
except NoNodeError:
break
for _ in iterate_timeout(10, "node to be deleted"):
try:
node.refresh(ctx)
except NoNodeError:
break
@simple_layout('layouts/nodepool.yaml', enable_nodepool=True)
def test_web_nodeset_list(self):
self.waitUntilSettled()

8
web/src/actions/adminActions.js
View File

@ -16,6 +16,7 @@ export const ADMIN_DEQUEUE_FAIL = 'ADMIN_DEQUEUE_FAIL'
export const ADMIN_ENQUEUE_FAIL = 'ADMIN_ENQUEUE_FAIL'
export const ADMIN_AUTOHOLD_FAIL = 'ADMIN_AUTOHOLD_FAIL'
export const ADMIN_PROMOTE_FAIL = 'ADMIN_PROMOTE_FAIL'
export const ADMIN_API_FAIL = 'ADMIN_API_FAIL'
function parseAPIerror(error) {
if (error.response) {
@ -46,4 +47,9 @@ export const addAutoholdError = error => ({
export const addPromoteError = error => ({
type: ADMIN_PROMOTE_FAIL,
notification: parseAPIerror(error)
})
})
export const addApiError = error => ({
type: ADMIN_API_FAIL,
notification: parseAPIerror(error)
})

16
web/src/api.js
View File

@ -306,10 +306,24 @@ function fetchNodesetRequests(apiPrefix) {
return makeRequest(apiPrefix + 'nodeset-requests')
}
function deleteNodesetRequest(apiPrefix, requestId) {
return makeRequest(
apiPrefix + '/nodeset-requests/' + requestId,
'delete'
)
}
function fetchNodes(apiPrefix) {
return makeRequest(apiPrefix + 'nodes')
}
function setNodeState(apiPrefix, requestId, state) {
return makeRequest(
apiPrefix + '/nodes/' + requestId,
'put',
{ state }
)
}
function fetchSemaphores(apiPrefix) {
return makeRequest(apiPrefix + 'semaphores')
}
@ -414,6 +428,7 @@ export {
buildImage,
deleteImageBuildArtifact,
deleteImageUpload,
deleteNodesetRequest,
dequeue,
dequeue_ref,
enqueue,
@ -454,5 +469,6 @@ export {
getLogFile,
getStreamUrl,
promote,
setNodeState,
setTenantState,
}

48
web/src/pages/Nodes.jsx
View File

@ -20,6 +20,7 @@ import {
TableVariant,
TableHeader,
TableBody,
ActionsColumn,
} from '@patternfly/react-table'
import * as moment from 'moment'
import * as moment_tz from 'moment-timezone'
@ -40,6 +41,9 @@ import {
} from '@patternfly/react-icons'
import { IconProperty } from '../Misc'
import { setNodeState } from '../api'
import { addNotification } from '../actions/notifications'
import { addApiError } from '../actions/adminActions'
import { fetchNodesIfNeeded } from '../actions/nodes'
import { Fetchable } from '../containers/Fetching'
@ -47,6 +51,7 @@ import { Fetchable } from '../containers/Fetching'
class NodesPage extends React.Component {
static propTypes = {
tenant: PropTypes.object,
user: PropTypes.object,
remoteData: PropTypes.object,
dispatch: PropTypes.func
}
@ -68,6 +73,23 @@ class NodesPage extends React.Component {
}
}
handleStateChange(nodeId, state) {
setNodeState(this.props.tenant.apiPrefix, nodeId, state)
.then(() => {
this.props.dispatch(addNotification(
{
text: 'Node state updated.',
type: 'success',
status: '',
url: '',
}))
this.props.dispatch(fetchNodesIfNeeded(this.props.tenant, true))
})
.catch(error => {
this.props.dispatch(addApiError(error))
})
}
render () {
const { remoteData } = this.props
const nodes = remoteData.nodes
@ -120,7 +142,11 @@ class NodesPage extends React.Component {
<IconProperty icon={<PencilAltIcon />} value="Comment" />
),
dataLabel: 'comment',
}
},
{
title: '',
dataLabel: 'action',
},
]
let rows = []
nodes.forEach((node) => {
@ -129,7 +155,7 @@ class NodesPage extends React.Component {
const state_time = typeof(node.state_time) === 'string' ?
moment_tz.utc(node.state_time) :
moment.unix(node.state_time)
let r = [
const r = [
{title: node.id, props: {column: 'ID'}},
{title: node.type.join(','), props: {column: 'Label' }},
{title: node.connection_type, props: {column: 'Connection'}},
@ -139,8 +165,23 @@ class NodesPage extends React.Component {
{title: state_time.fromNow(), props: {column: 'Age'}},
{title: node.comment, props: {column: 'Comment'}},
]
rows.push({cells: r})
if (node.uuid && this.props.user.isAdmin && this.props.user.scope.indexOf(this.props.tenant.name) !== -1) {
r.push({title:
<ActionsColumn items={[
{
title: 'Set to HOLD',
onClick: () => this.handleStateChange(node.uuid, 'hold')
},
{
title: 'Set to USED',
onClick: () => this.handleStateChange(node.uuid, 'used')
},
]}/>
})
}
rows.push({cells: r})
})
return (
<PageSection variant={PageSectionVariants.light}>
<PageSection style={{paddingRight: '5px'}}>
@ -168,4 +209,5 @@ class NodesPage extends React.Component {
export default connect(state => ({
tenant: state.tenant,
remoteData: state.nodes,
user: state.user,
}))(NodesPage)

41
web/src/pages/NodesetRequests.jsx
View File

@ -20,6 +20,7 @@ import {
TableVariant,
TableHeader,
TableBody,
ActionsColumn,
} from '@patternfly/react-table'
import * as moment_tz from 'moment-timezone'
import {
@ -38,6 +39,9 @@ import {
} from '@patternfly/react-icons'
import { IconProperty } from '../Misc'
import { deleteNodesetRequest } from '../api'
import { addNotification } from '../actions/notifications'
import { addApiError } from '../actions/adminActions'
import { fetchNodesetRequestsIfNeeded } from '../actions/nodesetRequests'
import { Fetchable } from '../containers/Fetching'
@ -45,6 +49,7 @@ import { Fetchable } from '../containers/Fetching'
class NodesetRequestsPage extends React.Component {
static propTypes = {
tenant: PropTypes.object,
user: PropTypes.object,
remoteData: PropTypes.object,
dispatch: PropTypes.func
}
@ -66,6 +71,23 @@ class NodesetRequestsPage extends React.Component {
}
}
handleDelete(requestId) {
deleteNodesetRequest(this.props.tenant.apiPrefix, requestId)
.then(() => {
this.props.dispatch(addNotification(
{
text: 'Nodeset request deleted.',
type: 'success',
status: '',
url: '',
}))
this.props.dispatch(fetchNodesetRequestsIfNeeded(this.props.tenant, true))
})
.catch(error => {
this.props.dispatch(addApiError(error))
})
}
render () {
const { remoteData } = this.props
const nodesetRequests = remoteData.requests
@ -113,11 +135,14 @@ class NodesetRequestsPage extends React.Component {
),
dataLabel: 'provider',
},
{
title: '',
dataLabel: 'action',
},
]
let rows = []
nodesetRequests.forEach((request) => {
console.log(request)
let r = [
const r = [
{title: request.uuid, props: {column: 'UUID'}},
{title: request.labels.join(','), props: {column: 'Labels' }},
{title: request.state, props: {column: 'State'}},
@ -126,6 +151,17 @@ class NodesetRequestsPage extends React.Component {
{title: request.pipeline_name, props: {column: 'Pipeline'}},
{title: request.job_name, props: {column: 'Job'}},
]
if (this.props.user.isAdmin && this.props.user.scope.indexOf(this.props.tenant.name) !== -1) {
r.push({title:
<ActionsColumn items={[
{
title: 'Delete',
onClick: () => this.handleDelete(request.uuid)
},
]}/>
})
}
rows.push({cells: r})
})
return (
@ -155,4 +191,5 @@ class NodesetRequestsPage extends React.Component {
export default connect(state => ({
tenant: state.tenant,
remoteData: state.nodesetRequests,
user: state.user,
}))(NodesetRequestsPage)

58
zuul/web/__init__.py
View File

@ -2394,6 +2394,32 @@ class ZuulWebAPI(object):
ret.append(ProviderNodeConverter.toDict(node))
return ret
@cherrypy.expose
@cherrypy.tools.save_params()
@cherrypy.tools.json_in()
@cherrypy.tools.json_out(content_type='application/json; charset=utf-8')
@cherrypy.tools.handle_options(allowed_methods=['PUT', ])
@cherrypy.tools.check_tenant_auth()
def nodes_put(self, tenant_name, tenant, auth, node_id):
node = self.zuulweb.nodes_cache.getItem(node_id)
if not node or node.tenant_name != tenant.name:
raise cherrypy.HTTPError(404, "Node not found")
body = cherrypy.request.json
state = body.get('state')
if state not in {node.State.HOLD, node.State.USED}:
raise cherrypy.HTTPError(400, 'Invalid request body')
# We just let the LockException propagate up if we can't lock
# it.
with self.zuulweb.createZKContext(None, self.log) as ctx:
with node.locked(ctx, blocking=False):
self.log.info(f'User {auth.uid} setting node '
f'{node_id} to {state}')
with node.activeContext(ctx):
node.setState(state)
cherrypy.response.status = 201
@cherrypy.expose
@cherrypy.tools.save_params()
@cherrypy.tools.json_out(content_type='application/json; charset=utf-8')
@ -2409,6 +2435,26 @@ class ZuulWebAPI(object):
ret.append(NodesetRequestConverter.toDict(request))
return ret
@cherrypy.expose
@cherrypy.tools.save_params()
@cherrypy.tools.json_out(content_type='application/json; charset=utf-8')
@cherrypy.tools.handle_options(allowed_methods=['DELETE', ])
@cherrypy.tools.check_tenant_auth()
def nodeset_requests_delete(self, tenant_name, tenant, auth, request_id):
request = self.zuulweb.requests_cache.getItem(request_id)
if not request or request.tenant_name != tenant.name:
raise cherrypy.HTTPError(404, "Request not found")
# We just let the LockException propagate up if we can't lock
# it.
with self.zuulweb.createZKContext(None, self.log) as ctx:
with request.locked(ctx, blocking=False):
self.log.info(f'User {auth.uid} deleting nodeset request '
f'{request_id}')
with request.activeContext(ctx):
request.delete(ctx)
cherrypy.response.status = 204
@cherrypy.expose
@cherrypy.tools.save_params()
@cherrypy.tools.handle_options()
@ -3117,8 +3163,20 @@ class ZuulWeb(object):
controller=api, action='labels')
route_map.connect('api', '/api/tenant/{tenant_name}/nodes',
controller=api, action='nodes')
route_map.connect('api',
'/api/tenant/{tenant_name}/'
'nodes/{node_id}',
controller=api,
conditions=dict(method=['PUT', 'OPTIONS']),
action='nodes_put')
route_map.connect('api', '/api/tenant/{tenant_name}/nodeset-requests',
controller=api, action='nodeset_requests')
route_map.connect('api',
'/api/tenant/{tenant_name}/'
'nodeset-requests/{request_id}',
controller=api,
conditions=dict(method=['DELETE', 'OPTIONS']),
action='nodeset_requests_delete')
route_map.connect('api', '/api/tenant/{tenant_name}/key/'
'{project_name:.*}.pub',
controller=api, action='key')