-
Notifications
You must be signed in to change notification settings - Fork 359
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add decompression flag to openstack_images_image_v2 resource #1482
Add decompression flag to openstack_images_image_v2 resource #1482
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for your PR. I left a couple of comments.
openstack/images_image_v2.go
Outdated
var reader io.ReadCloser | ||
decompress := d.Get("decompress").(bool) | ||
|
||
if decompress && !resp.Uncompressed { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
checking for resp.Uncompressed
doesn't make sense, since it is used only for Content-Encoding
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to check if we're not trying to decompress something already decompressed by golang stack or which was never compressed to begin with
In addition please update the
|
c4d7033
to
7fbd8cc
Compare
Did the modifications in the last version |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. LGTM. Please fix the comments. And I think it's ready to be merged.
7fbd8cc
to
cc2f094
Compare
Updated |
@spnngl can you fix golang linter issues? |
It also looks like OpenStack test env doesn't support web-download anymore. Can you add an exception and skip these tests for a while? |
ce7f87a
to
4f7becb
Compare
Pushed a new version with lint fix + web_download tests deactivated |
@kayrus any update? We really need this to be merged! 🙂 |
I re-triggered glance tests, once they pass, the PR can be merged. |
can you add a
terraform-provider-openstack/openstack/import_openstack_images_image_v2_test.go Lines 29 to 33 in bd2071b
|
4f7becb
to
bac0376
Compare
I added the decompress ignore + rebase on main |
Some image are compressed and only filling Content-Type header resulting if uploading a compressed image to openstack. With this we automatically detects and uncompress images in this case. Example: ``` $ curl -I "https://stable.release.flatcar-linux.net/amd64-usr/current/flatcar_production_openstack_image.img.bz2" HTTP/2 200 server: nginx date: Thu, 16 Feb 2023 15:54:03 GMT content-type: application/x-bzip2 content-length: 371535602 etag: "rq4xkz657ape" last-modified: Wed, 15 Feb 2023 18:48:35 GMT accept-ranges: bytes ```
OpenStack dev env seems to not support it anymore.
bac0376
to
fead509
Compare
Since I think recently opendev,org added some limits to pulling repos etc and as we also have our daily check failing with a timeout. I need to have a look to make sure that the timeout is due to limits and think of a way to bypass it. Im addressing this in #1497 |
@nikParasyr @ozerovandrei I was not aware about the behavior when changelog is changed. I think we can skip failed |
@kayrus the idea behind this behavior with changelog is that we update it only before a release (or after multiple MRs are changed) and the whole ci runs to make sure all services are ok. But it can be a bit annoying if changelog is updated on each MR (also often we have conflicts due to that). From my side this can be merged as well. The failing ci jobs are not related to this change. |
Some image are compressed and only filling Content-Type header resulting if uploading a compressed image to openstack.
With this we automatically detects and uncompress images in this case.
Example:
Resolves #1481