Issues: Install test does not test validity of packages (rpm/deb) #556

ssorenso · 2017-01-23T21:39:36Z

Fixes #494

Problem:
The test install algorithm to work with a more generic pool

Analysis:
I made the changes to make it so that the .deb and .rpm install tests
would do what they're supposed to do. Before this change, the .deb and
.rpm failures were not being properly tracked and would always fail due
to improper regExp handling of expected strings.

Tests:
This is a test. To validate that this executes properly, use the
.travis_yml file in its normal execution phasing.

@<reviewer_id>

What issues does this address?

Fixes #
WIP #
...

What's this change do?

Where should the reviewer start?

Any background context?

Fixes F5Networks#494 Problem: The test install algorithm to work with a more generic pool Analysis: I made the changes to make it so that the .deb and .rpm install tests would do what they're supposed to do. Before this change, the .deb and .rpm failures were not being properly tracked and would always fail due to improper regExp handling of expected strings. Tests: This is a test. To validate that this executes properly, use the .travis_yml file in its normal execution phasing.

richbrowne · 2017-01-24T15:24:45Z

.travis.yml

@@ -18,6 +18,8 @@ before_install:

 install:
    - pip install tox
+    - pip install ordereddict


The ordereddict and six packages are not required for the build.

Correct. I had forgotten that I had put this in travis. I'll remove them and recommit

richbrowne · 2017-01-24T16:18:04Z

f5-openstack-agent-dist/Docker/ubuntu/install_test/fetch_and_install_deps.py

+from collections import deque, namedtuple
+
+f5_sdk_rest_pattern = re.compile("^f5-sdk\s*=\s*(\d+\.\d+\.\d+)$")
+f5_sdk_rest_pattern = re.compile(


Why was this redefined? I am not sure that the f5-sdk-rest pattern will match what you want it to.

The pattern is not used anymore. Will remove.

richbrowne · 2017-01-24T16:31:13Z

f5-openstack-agent-dist/Docker/redhat/install_test/fetch_and_install_deps.py

+        match = dep_match_re.match(line)
+        if match:
+            groups = list(match.groups())
+            my_dep = ReqDetails(groups[0], groups[2], groups[3])


Is groups[0] the whole line? Or is this the package name?

I stumbled upon this too. Python2.7's re (at least) is behaving a little differently than I would expect here, and I had to work through it in the debugger. This was what I found:

Normally, groups[0] would be the whole match; however, in this case because I'm doing a match within a match, I get:

dep_match_re = re.compile('^((python|f5-sdk)[\w-]*)\s([<>=]{1,2})\s(\S+)')
dependent = 'python-six >= 1.10.2'
match = dep_match_re.search(dependent)
match
<_sre.SRE_Match object at 0x100e6cca8>
match.groups()
('python-six', 'python', '>=', '1.10.2')

0: 'python-six' is given by the ^((python|f5-sdk)[\w-]+)
1: 'python' is given by the internal expression (python|f5-sdk)
2: '>=' is given by the ([<>=]{1,2})
3: (\S+)

Python acts a little differently when you're doing match within a match... it's a little hairy and unfortunate that it behaves this way. I would, like what you're implying, expect it to be 0 - the full match 1 - the python-six 2 - the python|f5-sdk matching. It normally behaves this way, so for it to be different for a match-within-a-match was confusing.

richbrowne · 2017-01-24T16:36:01Z

f5-openstack-agent-dist/Docker/ubuntu/install_test/Dockerfile.trusty

 RUN apt-get install python-requests git curl -y --force-yes
+RUN apt-get install python-six

 COPY ./fetch_and_install_deps.py /



While you are at it you might as well change the way this is invoked so that invocation fails by using CMD or ENTRYPOINT.

Can do. Will grep for where else 'ENTRYPOINT' and 'CMD' is being used in-code and try and script-kitty. Hopefully, I'll have it working by the time you get in this afternoon.

richbrowne · 2017-01-24T16:37:08Z

f5-openstack-agent-dist/Docker/ubuntu/install_test/Dockerfile.trusty

@@ -4,7 +4,10 @@ FROM ubuntu:trusty
 RUN apt-get update -y && apt-get install software-properties-common -y
 RUN apt-add-repository cloud-archive:liberty


This should work. Would you remove the cloud-archive:mitaka and run this test with cloud-archive:liberty

Err... right. Will remove the additional apt-add-repository...

Confirmed to work with just liberty repo.

richbrowne

See inline comments. We can discuss this. I like what you have done to manage the dependencies.

Fixes F5Networks#494 Problem: The test install algorithm to work with a more generic pool Analysis: I made the changes to make it so that the .deb and .rpm install tests would do what they're supposed to do. Before this change, the .deb and .rpm failures were not being properly tracked and would always fail due to improper regExp handling of expected strings. Tests: This is a test. To validate that this executes properly, use the .travis_yml file in its normal execution phasing.

ssorenso · 2017-01-24T19:55:52Z

Forgot the travis change... please ignore this latest push...

…so/f5-openstack-agent into feature_packaging_testing_fix_494

Fixes F5Networks#494 Problem: The test install algorithm to work with a more generic pool Analysis: I made the changes to make it so that the .deb and .rpm install tests would do what they're supposed to do. Before this change, the .deb and .rpm failures were not being properly tracked and would always fail due to improper regExp handling of expected strings. Tests: This is a test. To validate that this executes properly, use the .travis_yml file in its normal execution phasing.

ssorenso · 2017-01-24T20:50:21Z

Okay... I think this is ready for review again... Changes made:

Made the redhat Docker reflect the ubuntu
Made the ubuntu Docker use the ENTRYPOINT setup
Fixed the yml to no longer reflect the pip installs that are not necessary
Removed the redundant deps pattern declaration/creation

Fixes F5Networks#494 Problem: The test install algorithm to work with a more generic pool Analysis: I made the changes to make it so that the .deb and .rpm install tests would do what they're supposed to do. Before this change, the .deb and .rpm failures were not being properly tracked and would always fail due to improper regExp handling of expected strings. Tests: This is a test. To validate that this executes properly, use the .travis_yml file in its normal execution phasing.

ssorenso · 2017-01-25T18:34:31Z

Failed in Travis...
Traceback (most recent call last):
File "/fetch_and_install_deps.py", line 198, in
main(sys.argv)
File "/fetch_and_install_deps.py", line 191, in main
fetch_agent_dependencies(dist_dir, version, release, pkg_fullname)
File "/fetch_and_install_deps.py", line 100, in fetch_agent_dependencies
my_dep = ReqDetails(*m.groups())
TypeError: new() takes exactly 4 arguments (5 given) -- /var/wdir/f5-openstack-agent-dist/scripts/get-version-release.py --version --release

Will have a fix shortly.

Fixes F5Networks#494 Problem: The test install algorithm to work with a more generic pool Analysis: I made the changes to make it so that the .deb and .rpm install tests would do what they're supposed to do. Before this change, the .deb and .rpm failures were not being properly tracked and would always fail due to improper regExp handling of expected strings. Tests: This is a test. To validate that this executes properly, use the .travis_yml file in its normal execution phasing.

Unsupported features page is incorrect

richbrowne reviewed Jan 24, 2017

View reviewed changes

richbrowne requested changes Jan 24, 2017

View reviewed changes

sorensF5 added 2 commits January 24, 2017 12:35

sorensF5 added 2 commits January 24, 2017 13:02

Merge branch 'feature_packaging_testing_fix_494' of github.com:ssoren…

d7ea8ff

…so/f5-openstack-agent into feature_packaging_testing_fix_494

richbrowne self-assigned this Jan 25, 2017

sorensF5 added 2 commits January 25, 2017 09:49

richbrowne merged commit 826529d into F5Networks:liberty Jan 25, 2017

ssorenso mentioned this pull request Jan 27, 2017

RPM package installation fails #494

Closed

ssorenso deleted the feature_packaging_testing_fix_494 branch October 6, 2017 20:28

zhang-shengping pushed a commit to zhang-shengping/f5-openstack-agent that referenced this pull request Feb 27, 2019

Merge pull request F5Networks#556 from mattgreene/liberty_issue531

cac0903

Unsupported features page is incorrect

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issues: Install test does not test validity of packages (rpm/deb) #556

Issues: Install test does not test validity of packages (rpm/deb) #556

ssorenso commented Jan 23, 2017

richbrowne Jan 24, 2017

ssorenso Jan 24, 2017

richbrowne Jan 24, 2017

ssorenso Jan 24, 2017

richbrowne Jan 24, 2017

ssorenso Jan 24, 2017

richbrowne Jan 24, 2017

ssorenso Jan 24, 2017

richbrowne Jan 24, 2017

ssorenso Jan 24, 2017

ssorenso Jan 24, 2017

richbrowne left a comment

ssorenso commented Jan 24, 2017

ssorenso commented Jan 24, 2017

ssorenso commented Jan 25, 2017

		@@ -4,7 +4,10 @@ FROM ubuntu:trusty
		RUN apt-get update -y && apt-get install software-properties-common -y
		RUN apt-add-repository cloud-archive:liberty

Issues: Install test does not test validity of packages (rpm/deb) #556

Issues: Install test does not test validity of packages (rpm/deb) #556

Conversation

ssorenso commented Jan 23, 2017

What issues does this address?

What's this change do?

Where should the reviewer start?

Any background context?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

richbrowne left a comment

Choose a reason for hiding this comment

ssorenso commented Jan 24, 2017

ssorenso commented Jan 24, 2017

ssorenso commented Jan 25, 2017