You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by GitBox <gi...@apache.org> on 2020/12/02 09:25:24 UTC

[GitHub] [cloudstack-primate] DaanHoogland opened a new issue #835: [FEATURE] server: update template to another template type

DaanHoogland opened a new issue #835:
URL: https://github.com/apache/cloudstack-primate/issues/835


   **Is your feature request related to a problem? Please describe.**
   A new feature implemented in https://github.com/apache/cloudstack/pull/3945 needs primate implementation
   
   **Describe the solution you'd like**
   A clear and concise description of what you want to happen.
   
   **Describe alternatives you've considered**
   A clear and concise description of any alternative solutions or features you've considered.
   
   **Additional context**
   Add any other context or screenshots about the feature request here.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack-primate] DaanHoogland commented on issue #835: [FEATURE] server: update template to another template type

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #835:
URL: https://github.com/apache/cloudstack-primate/issues/835#issuecomment-740487937


   The discussion is about reverting not about merging. I think @PaulAngus ' concerns about the backend change are valid though not confirmed. The frontend only exposes this. In addition to that @rhtyd and I discovered a discrepency that can easily be dealt with but needs addressing; when editting both the router checkbox and the template type dropdown are available. The router checkbox should be removed as the values of both can be conflicting.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack-primate] rhtyd closed issue #835: [FEATURE] server: update template to another template type

Posted by GitBox <gi...@apache.org>.
rhtyd closed issue #835:
URL: https://github.com/apache/cloudstack-primate/issues/835


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack-primate] ravening commented on issue #835: [FEATURE] server: update template to another template type

Posted by GitBox <gi...@apache.org>.
ravening commented on issue #835:
URL: https://github.com/apache/cloudstack-primate/issues/835#issuecomment-739547525


   @PaulAngus  if I'm not mistaken, even in backend, you can change template type from any to any


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack-primate] rhtyd commented on issue #835: [FEATURE] server: update template to another template type

Posted by GitBox <gi...@apache.org>.
rhtyd commented on issue #835:
URL: https://github.com/apache/cloudstack-primate/issues/835#issuecomment-740590743


   The feature is restricted to root admin, only for the update/edit template API/form and would require the admin to change the global setting to actually effect change of an env's systemvmtemplate. David's working on a small conditional to check a template upgraded to a routing one is registered/downloaded on all zones, rest we can accept. I've added a fix to not show both isrouting and template type dropdown. 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack-primate] DaanHoogland commented on issue #835: [FEATURE] server: update template to another template type

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #835:
URL: https://github.com/apache/cloudstack-primate/issues/835#issuecomment-739798948


   @PaulAngus , in your points above I only read items that come down to "the operator should know what they are doing". I hope I am not missing any. Is the UI bit only for admins, @ravening ? 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack-primate] ravening commented on issue #835: [FEATURE] server: update template to another template type

Posted by GitBox <gi...@apache.org>.
ravening commented on issue #835:
URL: https://github.com/apache/cloudstack-primate/issues/835#issuecomment-739811445


   > @PaulAngus , in your points above I only read items that come down to "the operator should know what they are doing". I hope I am not missing any. Is the UI bit only for admins, @ravening ? 
   
   @DaanHoogland yes only for admins


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack-primate] rhtyd commented on issue #835: [FEATURE] server: update template to another template type

Posted by GitBox <gi...@apache.org>.
rhtyd commented on issue #835:
URL: https://github.com/apache/cloudstack-primate/issues/835#issuecomment-739895282


   I'm okay with what @PaulAngus @DaanHoogland @weizhouapache @ravening decide, I'm okay if we decide revert it for now and visit this for 4.16.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack-primate] DaanHoogland commented on issue #835: [FEATURE] server: update template to another template type

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #835:
URL: https://github.com/apache/cloudstack-primate/issues/835#issuecomment-743055926


   I am moving thsi to 1.1 as it is going to be needing some finishing but the general jist is done for this release


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack-primate] PaulAngus commented on issue #835: [FEATURE] server: update template to another template type

Posted by GitBox <gi...@apache.org>.
PaulAngus commented on issue #835:
URL: https://github.com/apache/cloudstack-primate/issues/835#issuecomment-737104971


   We were in a code freeze yesterday.  please revert this commit


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack-primate] PaulAngus commented on issue #835: [FEATURE] server: update template to another template type

Posted by GitBox <gi...@apache.org>.
PaulAngus commented on issue #835:
URL: https://github.com/apache/cloudstack-primate/issues/835#issuecomment-739556047


   Thanks for clarifying @ravening, but that make might make it worse as:
   
   - The description does not suggest that the system VM template that was active is updated to inactive. - leaving the DB in an inconsistent state.
   - The testing description only says that only user -> system was tested.  
   - And I can't _see_ anything that looks like the backend operations like downloading system VM templates to all secondary storage pools. 
   - User VM templates are only supposed to be in one pool, so what happens there Cloudstack isn't going to be expecting user templates in all sec storage pools - hows deletion and GC going to work?
   - Builtin templates should be in all secondary storage pools as well. do they get downloaded?
   - Builtin templates also should be in all storage pools.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack-primate] PaulAngus commented on issue #835: [FEATURE] server: update template to another template type

Posted by GitBox <gi...@apache.org>.
PaulAngus commented on issue #835:
URL: https://github.com/apache/cloudstack-primate/issues/835#issuecomment-739914243


   As far as I'm concerned, a 'feature' was added to the UI when in a code freeze. 
   
   We're deliberately exposing through the UI, a backend change that IMO wasn't fully thought through (as detailed above) and was hardly tested.  Therefore, I don't think it should ever have been merged.
   
   The determination that the admin should know what they're doing doesn't hold water, because they are very unlikely to know the backend mechanisms of making template changes. and even if they did know, CloudStack definitely doesn't do what it should do in some cases, and there has been no defensive testing to ensure it does in the rest.
   
   But I can't be bothered to argue any more. 
   
   So merge it if you want.
   
   @DaanHoogland @weizhouapache @ravening


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack-primate] davidjumani commented on issue #835: [FEATURE] server: update template to another template type

Posted by GitBox <gi...@apache.org>.
davidjumani commented on issue #835:
URL: https://github.com/apache/cloudstack-primate/issues/835#issuecomment-737704136


   @PaulAngus The backend commit went in October, the UI component was merged a couple of days ago. Should the UI addition be reverted? That would make the feature unusable 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack-primate] rhtyd commented on issue #835: [FEATURE] server: update template to another template type

Posted by GitBox <gi...@apache.org>.
rhtyd commented on issue #835:
URL: https://github.com/apache/cloudstack-primate/issues/835#issuecomment-737704937


    We merged the UI PR yesterday based on David and Gabriels' review, as the backend PR was already merged in Oct.
   @PaulAngus to revert both frontend/backend PRs/commits see revert button at the bottom of any merged PR for ex https://github.com/apache/cloudstack-primate/pull/838#event-4055796316, or it would be polite to request Wei to send a revert PR for the backend PR https://github.com/apache/cloudstack/pull/3945 merged in Oct and Rakesh to send a revert the UI PR.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack-primate] PaulAngus commented on issue #835: [FEATURE] server: update template to another template type

Posted by GitBox <gi...@apache.org>.
PaulAngus commented on issue #835:
URL: https://github.com/apache/cloudstack-primate/issues/835#issuecomment-739807903


   I'm afraid that that is not accurate;
   
   You're assuming that the admin knows how the backend database stores template data, including system VM templates. 
   Also, if an admin converts a 'user' template to a built-in template, an admin can fully expect that **CloudStack**  knows what it is doing and handles all back-end changes, in the DB and physical ones such as copying templates to every sec store in every zone.
   
   Also, I see no tests to ensure that if a builtin or system template is created (which would therefore be in all zones) and is then changed to a user template, that template deletion and GC occurs properly.
   
   I really hope that I don't have to go on listing functional requirements and possible failure cases that would need to be covered..
   .


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack-primate] davidjumani edited a comment on issue #835: [FEATURE] server: update template to another template type

Posted by GitBox <gi...@apache.org>.
davidjumani edited a comment on issue #835:
URL: https://github.com/apache/cloudstack-primate/issues/835#issuecomment-737704136


   @PaulAngus The backend commit went in October, the UI component was merged a couple of days ago. Should the UI addition be reverted? That would make the feature unaccessible for a user / admin


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack-primate] PaulAngus commented on issue #835: [FEATURE] server: update template to another template type

Posted by GitBox <gi...@apache.org>.
PaulAngus commented on issue #835:
URL: https://github.com/apache/cloudstack-primate/issues/835#issuecomment-739542026


   The proposed UI change does not match the functionality described in the Backend PR which only allows changes between user and system. the UI screenshot shows changes to/from all types.
   
   ![image](https://user-images.githubusercontent.com/4810220/101288528-0aa18100-37ef-11eb-8dd5-dc853702aae9.png)
   
   The UI change needs reverting -admins can use the API.
   
   The [(https://github.com/apache/cloudstack/pull/3945)]  logic also looks to be flawed, as the description does not suggest that the system VM template that was active is updated to inactive.  - leaving the DB in an inconsistent state. 
   
   it **_really_** needs fixing or reverting.
   
   @rhtyd @DaanHoogland @weizhouapache @ravening 
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack-primate] PaulAngus edited a comment on issue #835: [FEATURE] server: update template to another template type

Posted by GitBox <gi...@apache.org>.
PaulAngus edited a comment on issue #835:
URL: https://github.com/apache/cloudstack-primate/issues/835#issuecomment-739556047


   Thanks for clarifying @ravening, but that make might make it worse as:
   
   - The description does not suggest that the system VM template that was active is updated to inactive. - leaving the DB in an inconsistent state.
   - The testing description only says that only user -> system was tested.  
   - And I can't _see_ anything that looks like the backend operations like downloading system VM templates to all secondary storage pools. 
   - User VM templates are only supposed to be in one pool, so what happens there? Cloudstack isn't going to be expecting user templates in all sec storage pools - hows deletion and GC going to work?
   - Builtin templates should be in all secondary storage pools as well. do they get downloaded?
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack-primate] rhtyd edited a comment on issue #835: [FEATURE] server: update template to another template type

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on issue #835:
URL: https://github.com/apache/cloudstack-primate/issues/835#issuecomment-737704937


    We merged the UI PR yesterday based on David and Gabriels' review, as the backend PR was already merged in Oct.
   @PaulAngus to revert both frontend/backend PRs/commits see revert button at the bottom of any merged PR for ex https://github.com/apache/cloudstack-primate/pull/838#event-4055796316, or it would be polite to request Wei to send a revert PR for the backend PR https://github.com/apache/cloudstack/pull/3945 merged in Oct and Rakesh to send a revert PR for UI changes.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack-primate] DaanHoogland commented on issue #835: [FEATURE] server: update template to another template type

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #835:
URL: https://github.com/apache/cloudstack-primate/issues/835#issuecomment-739901439


   me too, the issue is not with the UI but with the logic in the backend.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org