Skip to content
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

EE model changes #8644

Merged
merged 11 commits into from
Dec 10, 2020
Merged

EE model changes #8644

merged 11 commits into from
Dec 10, 2020

Conversation

jbradberry
Copy link
Contributor

SUMMARY

Make the remaining outstanding model changes needed for the implementation of execution environments.

related #7064

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME
  • API
AWX VERSION
awx: 15.0.1

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@AlanCoding
Copy link
Member

The PR shanemcd#19 already merged these changes into #8598, which is targeting #8030 (like this PR). I feel like we would do best to focus our energy into merging #8598, unless there's some reason we have to do a major course correction on that, which probably wouldn't go well.

@AlanCoding
Copy link
Member

Wait, that merge may have been undone with a force push.

@jbradberry
Copy link
Contributor Author

Wait, that merge may have been undone with a force push.

Right, we discussed it and decided that things were getting too unwieldy having everything building up in his PR, so we backed it out.

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@@ -27,6 +26,7 @@ class Meta:
verbose_name=_('image location'),
help_text=_("The registry location where the container is stored."),
)
pull = models.BooleanField(default=True)
Copy link
Member

Choose a reason for hiding this comment

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

I would like to see this put to use, or at least a plan of how it might be used, because it's ansible-runner that actually uses the podman/docker commands (or not even that, with openshift). Could we leave it out until we know more?

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@shanemcd shanemcd force-pushed the execution-environments branch 4 times, most recently from 3092bb9 to d4f60d8 Compare November 23, 2020 15:43
@shanemcd
Copy link
Member

Things went very sideways when I tried to rebase this PR, but I feel pretty confident about where I ended up. Sorry for the force-pushes.

The test that was failing on Friday is still failing today. Need to look into that.

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@shanemcd
Copy link
Member

2020-11-23 15:56:28.685312 | static | Traceback (most recent call last):
2020-11-23 15:56:28.685409 | static |   File "/awx_devel/awx/main/tests/unit/test_tasks.py", line 630, in test_options_jinja_usage
2020-11-23 15:56:28.685502 | static |     assert 'Jinja variables are not allowed' in update_model_call['result_traceback']
2020-11-23 15:56:28.685803 | static | AssertionError: assert 'Jinja variables are not allowed' in 'Traceback (most recent call last):\n  File "/awx_devel/awx/main/tasks.py", line 1413, in run\n    execution_environme...ror: Database access not allowed, use the "django_db" mark, or the "db" or "transactional_db" fixtures to enable it.\n'

@@ -95,6 +95,9 @@ class Meta:
job_template_admin_role = ImplicitRoleField(
parent_role='admin_role',
)
execution_environment_admin_role = ImplicitRoleField(
parent_role='admin_role',
)
Copy link
Member

Choose a reason for hiding this comment

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

With the addition of this, I'd expect access.py to switch a few things from the organization admin_role to the execution_environment_admin_role.

Q(organization__in=Organization.accessible_pk_qs(self.user, 'admin_role')) |

and

return Organization.accessible_objects(self.user, 'admin_role').exists()

and maybe?

if self.user not in obj.organization.admin_role:

that one might depend on intent

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@shanemcd shanemcd force-pushed the execution-environments branch 2 times, most recently from 9240c90 to 5917331 Compare November 30, 2020 15:43
@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@shanemcd shanemcd force-pushed the execution-environments branch 2 times, most recently from f1ef0eb to e169f63 Compare December 1, 2020 16:40
@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@@ -1160,6 +1159,37 @@ def create(self):
}
)

ManagedCredentialType(
namespace='registry',
kind='registry',
Copy link
Member

Choose a reason for hiding this comment

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

Because the credential is applied to the podman process, not the ansible-playbook process, I think it does make sense to add a new kind option of "registry" for pulling from an image registry.

A big "but" here - this doesn't actually add it as an option. You would need to add it to the KIND_CHOICES listing, and this PR does not do that now.

With the way that ansible-runner works... this might be accomplish-able by modifying os.environ, and not making other ansible-runner code changes. There are still a lot of question marks there.

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@softwarefactory-project-zuul
Copy link
Contributor

Merge Failed.

This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset.

@jbradberry jbradberry force-pushed the execution-environments branch from 1f99eb1 to 1884f27 Compare December 10, 2020 19:04
jbradberry and others added 11 commits December 10, 2020 14:05
- a new unique name field to EE
- a new configure-Tower-in-Tower setting DEFAULT_EXECUTION_ENVIRONMENT
- an Org-level execution_environment_admin_role
- a default_environment field on Project
- a new Container Registry credential type
- order EEs by reverse of the created timestamp
- a method to resolve which EE to use on jobs
as well as changes to other ones that need to be able to attach EEs.
so that it can be used with AdHocCommands as well.
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded (gate pipeline).

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit cf99057 into ansible:execution-environments Dec 10, 2020
@jbradberry jbradberry deleted the ee-model-changes branch December 10, 2020 20:10
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.

4 participants