Skip to content

Conversation

@scsynergy
Copy link

Description

This PR adds an object storage plugin for Huawei OBS, but also introduces an error I do not know how to fix when I
execute "mvn -pl :cloud-client-ui jetty:run":
http://192.168.17.252:8080/client/ returns "HTTP ERROR 503 Service Unavailable" because of the following exception:

[WARNING] Failed startup of context o.e.j.m.p.JettyWebAppContext@1df8ea34{/client,file:///opt/cloudstack-huawei-obs/client/target/classes/META-INF/webapp/,UNAVAILABLE}{file:///opt/cloudstack-huawei-obs/client/target/classes/META-INF/webapp/}
java.lang.NullPointerException

When I remove my module from client/pom.xml http://192.168.17.252:8080/client/ is available and I can log into the UI,
but my plugin is not registered as provider.

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • [ x] New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)
  • build/CI

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • [x ] Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

	added "Huawei OBS" to the UI provider dropdown
	added the plugin to the dependencies in client/pom.xml
		this causes [WARNING] Failed startup of context o.e.j.m.p.JettyWebAppContext@63893cf6{/client,file:///opt/cloudstack-huawei-obs/client/target/classes/META-INF/webapp/,UNAVAILABLE}{file:///opt/cloudstack-huawei-obs/client/target/classes/META-INF/webapp/}
java.lang.NullPointerException
		when doing mvn -pl :cloud-client-ui jetty:run
@boring-cyborg
Copy link

boring-cyborg bot commented Dec 15, 2023

Congratulations on your first Pull Request and welcome to the Apache CloudStack community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md)
Here are some useful points:

@shwstppr shwstppr marked this pull request as draft December 15, 2023 11:44
</modules>
<dependencies>
<dependency>
<dependency>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tab

Suggested change
<dependency>
<dependency>

@@ -0,0 +1,44 @@
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<!--
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.
-->
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">

@@ -0,0 +1,453 @@
package org.apache.cloudstack.storage.datastore.driver;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
package org.apache.cloudstack.storage.datastore.driver;
// 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.
package org.apache.cloudstack.storage.datastore.driver;

@@ -0,0 +1,111 @@
package org.apache.cloudstack.storage.datastore.lifecycle;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
package org.apache.cloudstack.storage.datastore.lifecycle;
// 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.
package org.apache.cloudstack.storage.datastore.lifecycle;

@@ -0,0 +1,64 @@
package org.apache.cloudstack.storage.datastore.provider;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
package org.apache.cloudstack.storage.datastore.provider;
// 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.
package org.apache.cloudstack.storage.datastore.provider;

@@ -0,0 +1,87 @@
package org.apache.cloudstack.storage.datastore.driver;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
package org.apache.cloudstack.storage.datastore.driver;
// 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.
package org.apache.cloudstack.storage.datastore.driver;

@@ -0,0 +1,34 @@
package org.apache.cloudstack.storage.datastore.provider;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
package org.apache.cloudstack.storage.datastore.provider;
// 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.
package org.apache.cloudstack.storage.datastore.provider;

@@ -0,0 +1,2 @@
name=storage-object-huawei-obs
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
name=storage-object-huawei-obs
# 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.
name=storage-object-huawei-obs

@@ -0,0 +1,12 @@
<beans xmlns="http://www.springframework.org/schema/beans"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<beans xmlns="http://www.springframework.org/schema/beans"
<!--
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.
-->
<beans xmlns="http://www.springframework.org/schema/beans"

@DaanHoogland DaanHoogland changed the title This is a draft pull request for Kishan adding Huawei OBS as object store. Jan 30, 2024
@codecov
Copy link

codecov bot commented Jan 30, 2024

Codecov Report

Attention: 281 lines in your changes are missing coverage. Please review.

Comparison is base (cfb4d43) 4.38% compared to head (34e1673) 29.29%.

❗ Current head 34e1673 differs from pull request most recent head 9f2cbec. Consider uploading reports for the commit 9f2cbec to get more accurate results

Files Patch % Lines
...tastore/driver/HuaweiObsObjectStoreDriverImpl.java 9.19% 243 Missing and 4 partials ⚠️
...e/lifecycle/HuaweiObsObjectStoreLifeCycleImpl.java 8.57% 32 Missing ⚠️
...ore/provider/HuaweiObsObjectStoreProviderImpl.java 84.61% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##              main    #8359       +/-   ##
============================================
+ Coverage     4.38%   29.29%   +24.91%     
- Complexity       0    32112    +32112     
============================================
  Files          361     5344     +4983     
  Lines        28706   375353   +346647     
  Branches      5004    54585    +49581     
============================================
+ Hits          1258   109961   +108703     
- Misses       27309   250717   +223408     
- Partials       139    14675    +14536     
Flag Coverage Δ
simulator-marvin-tests 22.54% <4.68%> (?)
uitests 4.39% <ø> (+0.01%) ⬆️
unit-tests 16.49% <9.68%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -0,0 +1,12 @@
<?xml version="1.0" encoding="UTF-8"?>
Copy link
Contributor

@DaanHoogland DaanHoogland Jan 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<?xml version="1.0" encoding="UTF-8"?>
<!--
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.
-->
<?xml version="1.0" encoding="UTF-8"?>

@@ -0,0 +1,9 @@
<?xml version="1.0" encoding="UTF-8"?>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<?xml version="1.0" encoding="UTF-8"?>
<!--
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.
-->
<?xml version="1.0" encoding="UTF-8"?>

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@scsynergy , I think we need to not add binary files in this repo. There is a construct for non-opensource files to be added in https://github.com/shapeblue/cloudstack-nonoss and then used if a project needs it.

@DaanHoogland
Copy link
Contributor

@scsynergy is your with that jar added? if so you probably want to add it as non-oss. (se my remark above)
Also note I gave your PR a more descriptive title (change if you don´t like it)
Feel free to ask the community anything here or on the mailing lists (or on a range of other platforms)

Copy link
Contributor

@JoaoJandre JoaoJandre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, @scsynergy please merge this PR with main and change the loggers names/imports. #7131 description has a script you can use if you want to (after merging with main).


public class HuaweiObsObjectStoreLifeCycleImpl implements ObjectStoreLifeCycle {

private static final Logger LOG = Logger.getLogger(HuaweiObsObjectStoreLifeCycleImpl.class);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private static final Logger LOG = Logger.getLogger(HuaweiObsObjectStoreLifeCycleImpl.class);
protected Logger logger = LogManager.getLogger(getClass());

Also need to update the log4j import

@scsynergy scsynergy closed this Feb 9, 2024
@DaanHoogland
Copy link
Contributor

@scsynergy did you close this on purpose? Are you going to open a new PR?

@DaanHoogland DaanHoogland added this to the unplanned milestone May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants