story#: Fix bug that readonly option doesn’t setup with volume create.

요약

참고

openstack volume create –size 1 –read-only <volume name> 명령어 수행 시 '–read-only' option을 주었지만 해당 option은 처리가 안되고 에러 메세지를 출력하며 option이 적용되지 않은 volume이 생성되는 버그

0. PR 전 버그 수정 과정

openstack volume create –size 1 –read-only <volume name> 명령어 수행 시 readonly option에 대한 에러가 처리되며 readonly option이 적용되지 않은 volume이 생성된 것을 다음과 같이 확인할 수 있다.

../../../_images/work2-1.png

해당 버그를 해결하기 위해 python-openstackcli.openstackclient.volume.v2.volume.py 파일 코드를 분석했습니다.

def update_readonly_flag(self, context, volume, flag):
     context.authorize(vol_action_policy.UPDATE_READONLY_POLICY,
                         target_obj=volume)
     if volume['status'] != 'available':
         msg = _('Volume %(vol_id)s status must be available '
                 'to update readonly flag, but current status is: '
                 '%(vol_status)s.') % {'vol_id': volume['id'],
                                       'vol_status': volume['status']}
         raise exception.InvalidVolume(reason=msg)
     self.update_volume_admin_metadata(context.elevated(), volume,
                                       {'readonly': str(flag)})
     LOG.info("Update readonly setting on volume "
              "completed successfully.",
              resource=volume)

if volume['status'] != 'available' : 해당 구문을 보면 volume.id 로 넘겨준 volume의 status가 available이 아니면 에러를 처리하게 되어 있다.

volume_client.volumes.create 으로 생성된 volume의 status가 바로 available이 되는 것이 아니라 잠시 creating 상태에 있기에 에러가 발생하며 –read-only 옵션이 처리가 되지 않았다.

이를 해결하기 위해 다음 코드와 같이 volume.py 에서 volume의 api 를 호출하여 상태 체크를 계속 해주었습니다.

# 추가 코드 1
while volume.status == "creating":
     volume = volume_client.volumes.get(volume.id)

if parsed_args.bootable or parsed_args.non_bootable:
     try:
         volume_client.volumes.set_bootable(
             volume.id, parsed_args.bootable)
     except Exception as e:
         LOG.error(_("Failed to set volume bootable property: %s"), e)
if parsed_args.read_only or parsed_args.read_write:
     try:
         volume_client.volumes.update_readonly_flag(
             volume.id,
             parsed_args.read_only)
     except Exception as e:
         LOG.error(_("Failed to set volume read-only access "
                     "mode flag: %s"), e)
# 추가 코드 2
volume = volume_client.volumes.get(volume.id)

해당 버그를 수정하니 또 다른 에러를 발견했다.

readonly 옵션으로 volume 인스턴스를 생성한 결과 출력되는 volume 정보에는 –read-only option이 빠져있었지만 다시 show 명령어로 확인한 결과 정상적으로 readonly option이 적용이 되어 있었다.

원인

출력해주는 volume 값을 바꾸지 않아 readonly 옵션을 주어도 생성 결과 출력값에는 빠져있었던 것이었다.

해결방안

모든 옵션을 처리하고 마지막에 volume api를 호출하여 volume 값을 업데이트 해주는 방향으로 코드를 수정했다.

수정 코드를 tox를 통해 test case를 돌리던 중 에러가 발생했다. 추가 코드 2 로 인해 발생하는 문제였다.

원인

test를 돌리면 fake volume object 의 get API 가 제대로 호출 되지 않아 에러가 발생했다.

해결방안
self.volumes_mock.get.return_value = self.new_volume

해당 명령어를 TestVolumeCreate class 의 setUp 메서드에 추가해주어 create fake_volume object 시 생성된 fake object 가 get API 를 호출할 수 있게끔 수정했다.

위와 같이 코드 수정을 마치고 첫번째 PR 을 올렸다.

1. 첫번째 PR 에 대한 리뷰 및 코드 수정

첫번째 PR에 maintainer 님이 코드 리뷰 후 comment를 달아주셨다.

참고

volume 의 status 가 'available' 이 될 때까지 while 문으로 묶어주면 무한 루프에 빠지는 위험이 있을 수 있으며 코드 상 보기에 깔끔하지 않아 기존에 만들어진 'wait_for_status' 메서드를 활용해서 코드를 수정하라는 피드백을 받았다.

if parsed_args.read_only or parsed_args.read_write:
     sys.stdout.write("Waiting for volume to be created.\n")
     if utils.wait_for_status(
             volume_client.volumes.get,
             volume.id,
             success_status=['available'],
             error_status=['error'],
             sleep_time=1
     ):
         try:
             volume_client.volumes.update_readonly_flag(
                 volume.id,
                 parsed_args.read_only)
         except Exception as e:
             LOG.error(_("Failed to set volume read-only access "
                         "mode flag: %s"), e)

     volume = volume_client.volumes.get(volume.id)

maintainer 님이 주신 피드백을 통해 코드를 다음과 같이 수정 후 문제 없이 잘 동작하는 것을 확인했다.

하지만 test case 를 돌리는 중 'wait_for_status' 메서드 내에서 무한루프 가 도는 문제점을 확인했다.

이를 위해 'wait_for_status' 메서드 내 코드를 분석했다.

def wait_for_status(status_f,
                    res_id,
                    status_field='status',
                    success_status=['active'],
                    error_status=['error'],
                    sleep_time=5,
                    callback=None):
     while True:
         res = status_f(res_id)
         status = getattr(res, status_field, '').lower()
         if status in success_status:
             retval = True
             break
         elif status in error_status:
             retval = False
             break
         if callback:
             progress = getattr(res, 'progress', None) or 0
             callback(progress)
         time.sleep(sleep_time)
     return retval

volume create API 는 volume 의 status 의 return 값을 'available', 'creating', 'error' 만 있다고 공식문서에서 정의하고 있다.

하지만 test case 에선 랜덤으로 'in_use' status 를 받아 wait_for_status 에서 무한루프가 도는 것이다. 'in_use' status 값을 갖는 이유는 FakeVolume class 내 create_one_volume 메서드 내에서 fake volume status 값을 'in_use', 'available' 을 랜던하게 갖게 한다.

해당 문제를 해결하고자 다음과 같이 코드를 수정했다.

# fakes.py
class FakeVolume(object):
     """Fake one or more volumes."""

     @staticmethod
     def create_one_volume(attrs=None,
                           status=['available', 'in_use']):
     ...

# test_volume.py
self.new_volume = volume_fakes.FakeVolume.create_one_volume(
             status=['available']
         )

FakeVolume 객체가 생성될 때 status 를 받을 수 있도록 코드를 수정하고 이 메서드를 호출할 때는 'available' 만 status 를 가질 수 있도록 코드를 수정했다.

이처럼 모든 에러를 처리하고 2번째 PR을 올렸으며 현재 maintainer 님의 review 를 기다리고 있다.

../../../_images/work2-2.png

Reference