Tejaskriya commented on code in PR #8112: URL: https://github.com/apache/ozone/pull/8112#discussion_r2003642862
########## hadoop-ozone/dist/src/main/smoketest/admincli/testdiskbalancer.robot: ########## @@ -0,0 +1,56 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +*** Settings *** +Documentation Smoketest ozone cluster startup +Library OperatingSystem +Library String +Library Collections +Resource ../commonlib.robot +Resource ../ozone-lib/shell.robot + +** Test Cases *** +Check admin permission to start diskbalancer Review Comment: We can add one test case for checking all the commands that need admin privilege, another test case for checking the commands that don't require the same ```suggestion Check failure with non-admin user to start, update and stop diskbalancer ``` All the `Requires admin privilege <command>` lines can be moved under this test ########## hadoop-ozone/dist/src/main/smoketest/admincli/testdiskbalancer.robot: ########## @@ -0,0 +1,56 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +*** Settings *** +Documentation Smoketest ozone cluster startup +Library OperatingSystem +Library String +Library Collections +Resource ../commonlib.robot +Resource ../ozone-lib/shell.robot + +** Test Cases *** +Check admin permission to start diskbalancer + Requires admin privilege ozone admin datanode diskbalancer start -a + # Test user has admin privileges, hence command should be executed successfully + Run Keyword Kinit test user testuser testuser.keytab + ${result} = Execute and Ignore Error ozone admin datanode diskbalancer start -t 0.0002 -a + Should Contain ${result} Start DiskBalancer on datanode(s) + +Check admin permission to stop diskbalancer + Requires admin privilege ozone admin datanode diskbalancer stop -a + # Test user has admin privileges, hence command should be executed successfully + Run Keyword Kinit test user testuser testuser.keytab + ${result} = Execute and Ignore Error ozone admin datanode diskbalancer stop -a + Should Contain ${result} Stopping DiskBalancer on datanode(s) + +Check admin permission to update diskbalancer configuration + Requires admin privilege ozone admin datanode diskbalancer update -t 0.0002 -a + # Test user has admin privileges, hence command should be executed successfully + Run Keyword Kinit test user testuser testuser.keytab + ${result} = Execute and Ignore Error ozone admin datanode diskbalancer update -t 0.0002 -a + Should Contain ${result} Update DiskBalancer Configuration on datanode(s) + +Check with non admin user is it able to read diskbalancer status + # Test user2 has no admin privileges but still command should be executed successfully + Run Keyword Kinit test user testuser2 testuser2.keytab + ${result} = Execute ozone admin datanode diskbalancer status + Should Contain ${result} Status result: + +Check with non admin user is it able to read diskbalancer report + # Test user2 has no admin privileges but still command should be executed successfully Review Comment: After the rearrangement suggested above, all these comment lines can be removed as the test name will be self explanatory. ########## hadoop-ozone/dist/src/main/smoketest/admincli/testdiskbalancer.robot: ########## @@ -0,0 +1,56 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +*** Settings *** +Documentation Smoketest ozone cluster startup +Library OperatingSystem +Library String +Library Collections +Resource ../commonlib.robot +Resource ../ozone-lib/shell.robot + +** Test Cases *** +Check admin permission to start diskbalancer + Requires admin privilege ozone admin datanode diskbalancer start -a + # Test user has admin privileges, hence command should be executed successfully + Run Keyword Kinit test user testuser testuser.keytab + ${result} = Execute and Ignore Error ozone admin datanode diskbalancer start -t 0.0002 -a + Should Contain ${result} Start DiskBalancer on datanode(s) + +Check admin permission to stop diskbalancer + Requires admin privilege ozone admin datanode diskbalancer stop -a + # Test user has admin privileges, hence command should be executed successfully + Run Keyword Kinit test user testuser testuser.keytab + ${result} = Execute and Ignore Error ozone admin datanode diskbalancer stop -a + Should Contain ${result} Stopping DiskBalancer on datanode(s) + +Check admin permission to update diskbalancer configuration + Requires admin privilege ozone admin datanode diskbalancer update -t 0.0002 -a + # Test user has admin privileges, hence command should be executed successfully + Run Keyword Kinit test user testuser testuser.keytab + ${result} = Execute and Ignore Error ozone admin datanode diskbalancer update -t 0.0002 -a + Should Contain ${result} Update DiskBalancer Configuration on datanode(s) + +Check with non admin user is it able to read diskbalancer status Review Comment: ```suggestion Check success with non-admin user for status and report diskbalancer ``` This test and the next one can be combined under one test. ########## hadoop-ozone/dist/src/main/smoketest/admincli/testdiskbalancer.robot: ########## @@ -0,0 +1,56 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +*** Settings *** +Documentation Smoketest ozone cluster startup +Library OperatingSystem +Library String +Library Collections +Resource ../commonlib.robot +Resource ../ozone-lib/shell.robot + +** Test Cases *** +Check admin permission to start diskbalancer + Requires admin privilege ozone admin datanode diskbalancer start -a + # Test user has admin privileges, hence command should be executed successfully + Run Keyword Kinit test user testuser testuser.keytab + ${result} = Execute and Ignore Error ozone admin datanode diskbalancer start -t 0.0002 -a + Should Contain ${result} Start DiskBalancer on datanode(s) + +Check admin permission to stop diskbalancer Review Comment: ```suggestion Check success with admin user for start, update and stop diskbalancer ``` This test we can have for start, update and stop command being run after doing a kinit with testuser. All commands should be successful ########## hadoop-ozone/dist/src/main/smoketest/admincli/testdiskbalancer.robot: ########## @@ -0,0 +1,56 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +*** Settings *** +Documentation Smoketest ozone cluster startup +Library OperatingSystem +Library String +Library Collections +Resource ../commonlib.robot +Resource ../ozone-lib/shell.robot Review Comment: ```suggestion ``` We are not using this in test cases, it can be removed ########## hadoop-ozone/dist/src/main/smoketest/admincli/testdiskbalancer.robot: ########## @@ -0,0 +1,56 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +*** Settings *** +Documentation Smoketest ozone cluster startup +Library OperatingSystem +Library String +Library Collections Review Comment: These libraries are also not used, they can be removed ```suggestion ``` -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
