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

Thomas Lamprecht t.lamprecht at proxmox.com
Fri Nov 23 11:25:53 CET 2018


On 11/23/18 11:01 AM, Dominik Csapak wrote:
> some comments inline

thanks for your review!

> 
> 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

a sorry about all of them, not sure why I did not see this...

> ...
>> +
>> +    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
will do
 

> ...
>> +    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
> 

will do

>> +        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'

sounds OK, but later is preferred here.

> 
>> +
>> +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?

it made initially but here I do not use it anymore, will split up

> 
> 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

for now I'll do above

> 
> or have one for both together

I do not like that to much, it intertwines both components unnecessarily
and not really helps in understanding what happens.

> 
> 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
> 

left over, will remove

>> +
>> +    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 ?

yes, will do

>> +        ],
>> +        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