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

Test proxy #17

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Test proxy #17

wants to merge 1 commit into from

Conversation

ehelms
Copy link
Member

@ehelms ehelms commented Dec 14, 2020

Working to replace https://github.com/theforeman/forklift/blob/master/bats/fb-katello-proxy.bats which I believe is a two part test and I would like to split it up. The first part is testing that a content proxy sync, and the second is that content is available which feels more like testing a client against a content proxy. The latter part is a set of additional testing that I would like to add after initial client testing goes into the code base.

@pytest.fixture
def content_smart_proxy(api, organization):
content_proxy_id = '2'
content_proxy = api.resource('smart_proxies').call('show', {'id': content_proxy_id})
Copy link
Member Author

Choose a reason for hiding this comment

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

@ekohl So I looked at picking the content proxy from the API... but the mirror property did not appear to be a capability and rather was a setting (which I didnt see in the API response).

Copy link
Member

@ekohl ekohl Dec 14, 2020

Choose a reason for hiding this comment

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

Ah right, we don't expose settings since they may contain secrets. For example, it can expose a username/password which we don't want to leak. I think we need a way to declare which settings we do expose (because they are safe).

Edit: can't we list here and check that it at least is a valid ID? You can probably list all the ones with the pulpcore feature and take the one with the highest ID.

Copy link
Member Author

Choose a reason for hiding this comment

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

Can we add mirror into the set of capabilities? I tried looking at the smart-proxy code but was not as clear to me where to do that so it got the right value.

Copy link
Member

Choose a reason for hiding this comment

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

@ehelms ehelms force-pushed the test-proxy branch 2 times, most recently from 1bc6b61 to 9119339 Compare January 7, 2021 13:49
tests/test_smart_proxy.py Outdated Show resolved Hide resolved
tests/test_smart_proxy.py Outdated Show resolved Hide resolved
tests/test_smart_proxy.py Outdated Show resolved Hide resolved
content_proxy = proxy
break

content_proxy_id = content_proxy['id']
Copy link
Member

Choose a reason for hiding this comment

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

If the proxy is not found, this raises a KeyError. You will hit this guaranteed on a Pulp 3 only setup where you need to search for a Pulpcore feature with the mirror setting set to true. I don't think settings are exposed (the are considered private) so we should figure out a different way to detect this. Perhaps it should be a capability (which I think is exposed).

content_proxy_id = content_proxy['id']

environments = api.resource('capsule_content').call('lifecycle_environments', {
'id': content_proxy['id']
Copy link
Member

Choose a reason for hiding this comment

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

Any reason you don't use content_proxy_id here?

Suggested change
'id': content_proxy['id']
'id': content_proxy_id

Comment on lines +42 to +44
return api.resource('organizations').call('index', {
'search': entities['organization_label']
})['results'][0]
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we can use get an org by label (show action) but https://theforeman.org/api/2.3/apidoc/v2/organizations/show.html doesn't really clarify this. I know in some places we can use some name instead of just the DB ID.

})['results'][0]


def test_smart_proxy_content_sync(api, content_smart_proxy):
Copy link
Member

Choose a reason for hiding this comment

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

This test will fail on a vanilla Foreman where Pulp is not present. That's something we need to deal with in a generic way. We'll see similar things with other plugins.

If we take REX: if present, it should be tested. If it isn't, it should be skipped.

That does leave the situation where a plugin should be present. If it's skipped, we need a way to fail in case it's missing. If we take the plugins testing pipeline, we should have some way to say "expect at least these plugins to be present or fail".

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.

2 participants