Skip to content

Changes prepared_image option for python_mod#65

Open
Maunty wants to merge 6 commits intomasterfrom
prepared_image
Open

Changes prepared_image option for python_mod#65
Maunty wants to merge 6 commits intomasterfrom
prepared_image

Conversation

@Maunty
Copy link
Member

@Maunty Maunty commented Apr 10, 2021

In case when we have long python_mod_req spec preprocess it whould be good to
have prepared image with all required staff intalled. Then bundle python
modules intallation step will become a copy procedure that will be much faster.
Feature is turned on by adding -i flag to adcm_sdk_pack launch.
Was refactored mechanism naming of modified image. Without -i option modified
image is always build. With it - packer try to find prepared image that
corespond to current hash of base image name and requirements.yaml, if image is
not found then it is prepared and not removed.

Example:

     - type: python_mod_req
       requirements: requirements.yml
       image: arenadata/adcm:2020013015
       target_dir: ansible

@Maunty Maunty requested a review from vgorkavenko April 10, 2021 07:16
@Maunty Maunty force-pushed the prepared_image branch 4 times, most recently from 0e4285f to 186a1fc Compare April 10, 2021 19:29
@Maunty
Copy link
Member Author

Maunty commented Apr 11, 2021

@vgorkavenko To be honest i dont like this implementation because we need 2 actions to bring this feature work.
I think that it will be better to change algorithm to something like like that:

  • Get base image version
  • Get requirements.yaml hash
  • Compute some uniq tag for this case.
    In this case this option can be used even on CI because this will be prepared image validation algorithm.
    What do u think?

@vgorkavenko
Copy link
Contributor

@Maunty In this implementation, it seems to me that Flags class is superfluous. If, in your opinion, this is necessary, then you need to give it a more informative name, since "Flags" is too general. Also I think prepare_image option should take image name (and not take that name from spec.yaml). As for algorithm, I doubt that my opinion can somehow help you, since I don't know much about packaging bundles and processes in CI :)

@Maunty Maunty changed the title WIP Changes prepared_image option for python_mod Changes prepared_image option for python_mod Apr 12, 2021
@Maunty Maunty requested review from dgusakov and skhomuti April 12, 2021 18:46
# choose image where to install python pkgs
# by default adcm:latest but i think this may be bad practice
# better to use adcm_min_version of bundle
image_name = kwargs.get("image", "arenadata/adcm:latest")
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
image_name = kwargs.get("image", "arenadata/adcm:latest")
image_name = kwargs.get("image", "hub.arenadata.io/adcm/adcm:latest")

Copy link
Member Author

Choose a reason for hiding this comment

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

@dgusakov I removed this variable. Using a default image instead of defined one will lead to a unpredictable result in long terms.

Comment on lines +65 to +66
"adds prepared image behaviour to bundle`s spec"
"python_mod_req proccessing function"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any external description for this "behavior"?
If not consider extending this help

Maunty added 2 commits April 13, 2021 12:11
In case when we have long python_mod_req spec preprocess it whould be good to
have prepared image with all required staff intalled. Then bundle python
modules intallation step will become a copy procedure that will be much faster.
Feature is turned on by adding -i flag to adcm_sdk_pack launch.
Was refactored mechanism naming of modified image. Without -i option modified
image is always build. With it - packer try to find prepared image that
corespond to current hash of base image name and requirements.yaml, if image is
not found then it is prepared and not removed.

Example:
```yaml
     - type: python_mod_req
       requirements: requirements.yml
       image: arenadata/adcm:2020013015
       target_dir: ansible
```
Added autoescape for jinja template. To prevent expoloit code via template
injection.
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants