[pve-devel] [PATCH manager v2 5/5] ui: add CephFS integration

Dominik Csapak d.csapak at proxmox.com
Fri Nov 23 11:01:47 CET 2018


some comments inline

On 11/22/18 8:34 PM, Thomas Lamprecht wrote:
> create/destroy MDS and create CephFS (if none is configured yet).
> Can be improved, e.g., start/stop/restart for MDS this should be enough for a
> starter, though.
> 
> Basic code and ui layout is based off my dc/Cluster view. We may want to split
> the two grids out in separate defines, it could be a bit much to have all
> inline.
> 
> Signed-off-by: Thomas Lamprecht <t.lamprecht at proxmox.com>
> ---
> 
> new in v2
> 
>   www/manager6/Makefile       |   1 +
>   www/manager6/ceph/FS.js     | 385 ++++++++++++++++++++++++++++++++++++
>   www/manager6/node/Config.js |   8 +
>   3 files changed, 394 insertions(+)
>   create mode 100644 www/manager6/ceph/FS.js
> 
> diff --git a/www/manager6/Makefile b/www/manager6/Makefile
> index d005d714..e75f0de6 100644
> --- a/www/manager6/Makefile
> +++ b/www/manager6/Makefile
> @@ -93,6 +93,7 @@ JSSRC= 				                 	\
>   	panel/IPSet.js					\
>   	panel/ConfigPanel.js				\
>   	grid/BackupView.js				\
> +	ceph/FS.js					\
>   	ceph/Pool.js					\
>   	ceph/OSD.js					\
>   	ceph/Monitor.js					\
> diff --git a/www/manager6/ceph/FS.js b/www/manager6/ceph/FS.js
> new file mode 100644
> index 00000000..f2743a4d
> --- /dev/null
> +++ b/www/manager6/ceph/FS.js
> @@ -0,0 +1,385 @@
> +/*jslint confusion: true */
> +Ext.define('PVE.CephCreateFS', {
> +    extend: 'Proxmox.window.Edit',
> +    alias: 'widget.pveCephCreateFS',
> +
> +    showTaskViewer: true,
> +    //onlineHelp: 'pve_ceph_fs',
> +
> +    subject: 'Ceph FS',
> +    isCreate: true,
> +    method: 'POST',
> +
> +    setFSName: function(fsName) {
> +        var me = this;
wrong indentation
> +
> +	if (fsName === '') {
> +	    fsName = 'cephfs';
> +	}
> +
> +        me.url = "/nodes/" + me.nodename + "/ceph/fs/" + fsName;
wrong indentation
> +    },
> +
> +    items: [
> +	{
> +	    xtype: 'textfield',
> +	    fieldLabel: gettext('Name'),
> +	    name: 'name',
> +	    value: 'cephfs',
> +	    listeners: {
> +	        change: function(f, value) {
> +		    this.up('pveCephCreateFS').setFSName(value);
> +	        }
wrong indentation
> +	    },
> +	    submitValue: false, // already encoded in apicall URL
> +	    emptyText: 'cephfs'
> +	},
> +	{
> +	    xtype: 'proxmoxintegerfield',
> +	    fieldLabel: 'pg_num',
> +	    name: 'pg_num',
> +	    value: 64,
> +	    emptyText: 64,
> +	    minValue: 8,
> +	    maxValue: 32768,
> +	    allowBlank: false
> +	},
> +	{
> +	    xtype: 'proxmoxcheckbox',
> +	    fieldLabel: gettext('Add Storage'),
> +	    value: true,
> +	    name: 'add_storage'
> +	}
> +    ],
> +
> +    initComponent : function()
> +        var me = this;

wrong indentation

> +
> +	if (!me.nodename) {
> +	    throw "no node name specified";
> +	}
> +
> +        Ext.apply(me, {
> +	    url: "/nodes/" + me.nodename + "/ceph/fs/cephfs",

you could do a me.setFSName(); instead, this way you only have one
location where you define the (default) path

> +	    defaults: {
> +		nodename: me.nodename
> +	    }
> +        });
> +
> +        me.callParent();
wrong indentation
> +    }
> +});
> +
> +Ext.define('PVE.CephCreateMDS', {
> +    extend: 'Proxmox.window.Edit',
> +    alias: 'widget.pveCephCreateMDS',
> +
> +    showProgress: true,
> +    //onlineHelp: 'pve_ceph_mds',
> +
> +    subject: 'Ceph MDS',
> +    isCreate: true,
> +    method: 'POST',
> +
> +    setNode: function(nodename) {
> +        var me = this;
> +
> +	me.nodename = nodename;
> +        me.url = "/nodes/" + nodename + "/ceph/mds/" + nodename;
> +    },
wrong indentation
> +
> +    items: [
> +	{
> +	    xtype: 'pveNodeSelector',
> +	    fieldLabel: gettext('Node'),
> +	    selectCurNode: true,
> +	    submitValue: false,
> +	    allowBlank: false,
> +	    listeners: {
> +		change: function(f, value) {
> +		    this.up('pveCephCreateMDS').setNode(value);
> +		}
> +	    }
> +	}
> +    ],
> +
> +    initComponent : function() {
> +        var me = this;
> +
> +	if (!me.nodename) {
> +	    throw "no node name specified";
> +	}
> +
> +        Ext.apply(me, {
> +	    url: "/nodes/" + me.nodename + "/ceph/mds/" + me.nodename
> +        });
> +

also here you could call me.setNode() for the same benefits
as above

> +        me.callParent();
> +    }
> +});

we could try to refactor CephCreateMon to something like
CephCreateService, since they look almost exactly the same

but we can still do this when/if we add 'create manager'

> +
> +Ext.define('PVE.NodeCephFSPanel', {
> +    extend: 'Ext.panel.Panel',
> +    xtype: 'pveNodeCephFSPanel',
> +    mixins: ['Proxmox.Mixin.CBind'],

high level comment to this panel

does the division of controllers/viewmodel really make sense?

we have a global viewmodel which gets passed to one
and inherited to the other panel, but we have 2 distinct viewcontrollers

in the global viewmodel we have 2 properties, where
each only gets access/set by one of the panels?
and mdscount only ever gets set?
or am i missing something here?

i think this makes the whole thing rather confusing
which panel can access which things in the viewmodel

i would propose either:

have each panel have its own controller/viewmodel

or have one for both together

additionally i think we should try to refactor the mds list
and mon list to a 'cephservicelist' as soon as
we add a managerlist, so the each panel has its own
controller and viewmodel would make more sense

> +
> +    title: gettext('Cluster Administration'),
> +    onlineHelp: 'chapter_pvecm',
> +
> +    border: false,
> +    defaults: {
> +	border: false,
> +	cbind: {
> +	    nodename: '{nodename}'
> +	}
> +    },
> +
> +    viewModel: {
> +	parent: null,
> +	data: {
> +	    cephfsConfigured: false,
> +	    mdscount: 0
> +	}
> +    },
> +
> +    /*initComponent: function() {
> +        var me = this;
> +        Ext.apply(me, {
> +	    defaults: {
> +		nodename: me.nodename,
> +		border: false
> +	    }
> +	});
> +
> +        me.callParent();
> +    },*/

is this meant to be still there?
if yes, i would prefer a small comment why

> +
> +    items: [
> +	{
> +	    xtype: 'grid',
> +	    title: gettext('CephFS'),
> +	    controller: {
> +		xclass: 'Ext.app.ViewController',
> +
> +		init: function(view) {
> +		    view.rstore = Ext.create('Proxmox.data.UpdateStore', {
> +			autoLoad: true,
> +			xtype: 'update',
> +			interval: 5 * 1000,
> +			autoStart: true,
> +			storeid: 'pve-ceph-fs',
> +			model: 'pve-ceph-fs'
> +		    });
> +		    view.setStore(Ext.create('Proxmox.data.DiffStore', {
> +			rstore: view.rstore,
> +			sorters: {
> +			    property: 'name',
> +			    order: 'DESC'
> +			}
> +		    }));
> +		    Proxmox.Utils.monStoreErrors(view, view.rstore);
> +		    view.rstore.on('load', this.onLoad, this);
> +		    view.on('destroy', view.rstore.stopUpdate);
> +		},
> +
> +		onCreate: function() {
> +		    var view = this.getView();
> +		    view.rstore.stopUpdate();
> +		    var win = Ext.create('PVE.CephCreateFS', {
> +			autoShow: true,
> +			nodename: view.nodename,
> +			listeners: {
> +			    destroy: function() {
> +				view.rstore.startUpdate();
> +			    }
> +			}
> +		    });
> +		},
> +
> +		onLoad: function(store, records, success) {
> +		    var vm = this.getViewModel();
> +		    if (!(success && records && records.length > 0)) {
> +			vm.set('cephfsConfigured', false);
> +			return;
> +		    }
> +		    vm.set('cephfsConfigured', true);
> +		}
> +	    },
> +	    tbar: [
> +		{
> +		    text: gettext('Create CephFS'),
> +		    reference: 'createButton',
> +		    handler: 'onCreate',
> +		    bind: {
> +			// only one CephFS per Ceph cluster makes sense for now
> +			disabled: '{cephfsConfigured}'
> +		    }
> +		}
> +	    ],
> +	    columns: [
> +		{
> +		    header: gettext('Name'),
> +		    flex: 1,
> +		    dataIndex: 'name'
> +		},
> +		{
> +		    header: 'Data Pool',
> +		    flex: 1,
> +		    dataIndex: 'data_pool'
> +		},
> +		{
> +		    header: 'Metadata Pool',
> +		    flex: 1,
> +		    dataIndex: 'metadata_pool'
> +		}
> +	    ],
> +	    cbind: {
> +		nodename: '{nodename}'
> +	    }
> +	},
> +	{
> +	    xtype: 'grid',
> +	    title: gettext('Metadata Servers'),
> +	    viewModel: {
> +		data: {
> +		    rowSelected: false
> +		}
> +	    },
> +	    controller: {
> +		xclass: 'Ext.app.ViewController',
> +
> +		init: function(view) {
> +		    view.rstore = Ext.create('Proxmox.data.UpdateStore', {
> +			autoLoad: true,
> +			xtype: 'update',
> +			interval: 3 * 1000,
> +			autoStart: true,
> +			storeid: 'pve-ceph-mds',
> +			model: 'pve-ceph-mds'
> +		    });
> +		    view.setStore(Ext.create('Proxmox.data.DiffStore', {
> +			rstore: view.rstore,
> +			sorters: {
> +			    property: 'id',
> +			    order: 'DESC'
> +			}
> +		    }));
> +		    Proxmox.Utils.monStoreErrors(view, view.rstore);
> +		    view.rstore.on('load', this.onLoad, this);
> +		    view.on('destroy', view.rstore.stopUpdate);
> +
> +		    var vm = this.getViewModel();
> +		    view.mon(view.selModel, "selectionchange", function() {
> +			var rec = view.selModel.getSelection()[0];
> +
> +			vm.set('rowSelected', !!rec);
> +		    });
> +		},
> +
> +		onCreateMDS: function() {
> +		    var view = this.getView();
> +		    view.rstore.stopUpdate();
> +		    var win = Ext.create('PVE.CephCreateMDS', {
> +			autoShow: true,
> +			nodename: view.nodename,
> +			listeners: {
> +			    destroy: function() {
> +				view.rstore.startUpdate();
> +			    }
> +			}
> +		    });
> +		},
> +
> +		onDestroyMDS: function() {
> +		    var view = this.getView();
> +		    var rec = view.selModel.getSelection()[0];
> +
> +		    if (!rec.data.host) {
> +			Ext.Msg.alert(gettext('Error'), "entry has no host");
> +			return;
> +		    }
> +
> +		    Proxmox.Utils.API2Request({
> +			url: "/nodes/" + rec.data.host + "/ceph/mds/" + rec.data.name,
> +			method: 'DELETE',
> +			success: function(response, options) {
> +			    var upid = response.result.data;
> +			    var win = Ext.create('Proxmox.window.TaskProgress', { upid: upid });
> +			    win.show();
> +			},
> +			failure: function(response, opts) {
> +			    Ext.Msg.alert(gettext('Error'), response.htmlStatus);
> +			}
> +		    });
> +		},
> +
> +		onLoad: function(store, records, success) {
> +		    var vm = this.getViewModel();
> +		    if (!success || !records) {
> +			vm.set('mdscount', 0);
> +			return;
> +		    }
> +		    vm.set('mdscount', records.length);
> +		}
> +	    },
> +	    tbar: [
> +		{
> +		    text: gettext('Create MDS'),
> +		    reference: 'createButton',
> +		    handler: 'onCreateMDS'
> +		},
> +		{
> +		    text: gettext('Destroy MDS'),
> +		    bind: {
> +			disabled: '{!rowSelected}'
> +		    },
> +		    handler: 'onDestroyMDS'
> +		}

could you not replace that with a proxmoxstdremovebtn ?
> +	    ],
> +	    columns: [
> +		{
> +		    header: gettext('Name'),
> +		    flex: 1,
> +		    dataIndex: 'name'
> +		},
> +		{
> +		    header: gettext('Host'),
> +		    flex: 1,
> +		    dataIndex: 'host'
> +		},
> +		{
> +		    header: gettext('Address'),
> +		    flex: 1,
> +		    dataIndex: 'addr'
> +		},
> +		{
> +		    header: gettext('State'),
> +		    flex: 1,
> +		    dataIndex: 'state'
> +		}
> +	    ],
> +	    cbind: {
> +		nodename: '{nodename}'
> +	    }
> +	}
> +    ]
> +}, function() {
> +    Ext.define('pve-ceph-mds', {
> +	extend: 'Ext.data.Model',
> +	fields: [ 'name', 'host', 'addr', 'state' ],
> +	proxy: {
> +	    type: 'proxmox',
> +	    url: "/api2/json/nodes/localhost/ceph/mds"
> +	},
> +	idProperty: 'name'
> +    });
> +    Ext.define('pve-ceph-fs', {
> +	extend: 'Ext.data.Model',
> +	fields: [ 'name', 'data_pool', 'metadata_pool' ],
> +	proxy: {
> +	    type: 'proxmox',
> +	    url: "/api2/json/nodes/localhost/ceph/fs"
> +	},
> +	idProperty: 'name'
> +    });
> +});
> diff --git a/www/manager6/node/Config.js b/www/manager6/node/Config.js
> index 8b2b802a..f9a62670 100644
> --- a/www/manager6/node/Config.js
> +++ b/www/manager6/node/Config.js
> @@ -340,6 +340,14 @@ Ext.define('PVE.node.Config', {
>   		    groups: ['ceph'],
>   		    itemId: 'ceph-osdtree'
>   		},
> +		{
> +		    xtype: 'pveNodeCephFSPanel',
> +		    title: 'CephFS',
> +		    iconCls: 'fa fa-folder',
> +		    groups: ['ceph'],
> +		    nodename: nodename,
> +		    itemId: 'ceph-cephfspanel'
> +		},
>   		{
>   		    xtype: 'pveNodeCephPoolList',
>   		    title: 'Pools',
> 





More information about the pve-devel mailing list