-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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 package versions as well as hashes #8360
Conversation
35deec6
to
cc20232
Compare
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: justinsb The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
nodeup/pkg/model/docker_test.go
Outdated
if version != expectedVersion { | ||
return fmt.Errorf("unexpected version, actual=%q, expected=%q", version, expectedVersion) | ||
} | ||
} else if strings.HasSuffix(u, ".tgz") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Packages for conatinerd have a tar.gz
suffix:
https://github.com/kubernetes/kops/pull/8199/files#diff-ecf61990c0cdda1c13ad07df0440a537R116-R135
} else if strings.HasSuffix(u, ".tgz") { | |
} else if strings.HasSuffix(u, ".tgz") || strings.HasSuffix(u, ".tar.gz") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call - added!
nodeup/pkg/model/docker_test.go
Outdated
return fmt.Errorf("did not expect version for tgz package") | ||
} | ||
} else { | ||
return fmt.Errorf("unexpected suffix for file (not .rpm or .deb)") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe also list .tgz
and .tar.gz
and the file name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks - done!
cc20232
to
424b7d4
Compare
Extend the existing "unit" test to check package versions, because some of the docker packages now have a '5:' prefix. Also correct the package versions that didn't have the prefix.
424b7d4
to
1d58f16
Compare
/lgtm |
/retest |
Extend the existing "unit" test to check package versions, because
some of the docker packages now have a '5:' prefix.