have origin.spec use hack scripts to build by tdawson · Pull Request #10398 · openshift/origin

tdawson · 2016-08-12T22:55:56Z

Use the hack scripts to build the binaries for the origin rpm

liggitt · 2016-08-12T22:59:42Z

Can you verify the Linux oc built by this shows GSSAPI in oc version?

stevekuznetsov · 2016-08-12T23:09:45Z

.tito/lib/origin/builder/__init__.py

            # builder
-            cmd = '. ./hack/common.sh ; echo $(os::build::ldflags)'
+
+            ## Fixup os_git_vars


We have os::build::os_version_vars in hack/common.sh -- is it possible to reuse that logic?

The big difference is that os::build::ldflags echo's everything out in one line. os::build::os_version_vars just set's variables. But I'll give it a try.

You could also write a wrapper:

function os::build::emit_os_version_vars() { os::build::os_version_vars echo "OS_GIT_COMMIT=${OS_GIT_COMMIT}...." }

Either way, I think it's worthwhile to keep the logic in one place so it can't drift. I found a bug in the logic this week, if there ends up a bug fix I'd rather it propagate correctly.

I totally agree with you, in keeping the logic in one place.
I wish the tito scripts were in bash so I could write a wrapper function. What I have below isn't pretty, but it get's the job done.

Sorry, I meant if you write a Bash wrapper in hack/common.sh and call it the same way you're calling os::build::ldflags.

Well, actually not the same way. You don't need to do echo $(funcname), you can simply do funcname. But that's a nitpick

I now understand what you are saying, but I'd like to keep the logic where it is for this pull request.

I have some ideas for building srpm's, #7268 and I'd like to defer this cleanup until then.

Sure thing. Your logic is distinct from the logic used in os::build::os_version_vars but I assume it's compatible for now.

tdawson · 2016-08-13T00:11:49Z

@liggitt Yep, looks like it does

$ _output/local/bin/linux/amd64/oc version
oc v1.3.0-alpha.3+ea4d441-dirty
kubernetes v1.3.0+507d3a7
features: Basic-Auth GSSAPI Kerberos SPNEGO

tdawson · 2016-08-13T01:42:24Z

[test]

Have tito update os_git_vars in origin.spec Set pkgdir the same as go_pkg_dir Use hack/build-cross.sh to build rpm binaries Install binaries from new build locations Add OS_GIT_VERSION to os_git_vars Fix ldflag generation by exporting OS_ROOT manually Do not export OS_ROOT Create extended test with scripts Remove ldflags since they are no longer needed Set OS_ROOT each time we use os::build::ldflags Use hack/common.h to set os_git_vars

openshift-bot · 2016-08-15T21:57:22Z

Evaluated for origin test up to 7a3a9d6

openshift-bot · 2016-08-15T23:27:20Z

continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/7935/)

tdawson · 2016-08-17T14:31:21Z

[test]

openshift-bot · 2016-08-17T14:32:13Z

The Origin test job could not be run again for this pull request.

If the proposed changes in this pull request caused the job to fail, update this pull request with new code to fix the issue(s).
If flaky tests caused the job to fail, leave a comment with links to the GitHub issue(s) in the openshift/origin repository with the kind/test-flake label that are tracking the flakes. If no issue already exists for the flake you encountered, create one.
If something else like CI system downtime or maintenance caused the job to fail, contact a member of the Team Project Committers group to trigger the job again.

tdawson · 2016-08-18T13:33:26Z

It looks like the tests failing were a false falure. Can we get a merge on this.

stevekuznetsov · 2016-08-18T13:40:00Z

@tdawson if we want green tests, just link to the conformance flake that you had and you can re-test. That being said, are there any tests in our CI right now that would prove correctness for what you've done? I don't think so. If not, there's no point in testing again and I'll merge it

sdodson · 2016-08-18T14:36:15Z

There's hack/test-rpm.sh but I don't believe it's wired in anywhere and likely is out of date.

stevekuznetsov · 2016-08-18T15:35:39Z

There's even a Vagrant command wired up for it but I don't think we ever call it. @tdawson can you open an issue to track the migration of the logic from in here to Bash? [merge]

openshift-bot · 2016-08-18T15:37:26Z

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/8149/) (Image: devenv-rhel7_4867)

openshift-bot · 2016-08-18T15:37:26Z

Evaluated for origin merge up to 7a3a9d6

tdawson · 2016-08-18T16:18:29Z

I ran the the hack/test-rpm.sh on this pull request.
rpm built fine.
rpmlint failed with 37 errors, 66 warnings.
All of the errors were false positives. Most of the errors are because we have non-standard permissions on things, and the usual errors that go binaries show in rpmlint.
Majority of the warning were because we didn't have man pages for all our binaries. Next highest were more go binary related. And then a smattering of others.

None of the errors or warning had anything that this pull request would have affected.

I think the reason this wasn't incorporated into the tests was due to the amount of cleanup the rpmlint is going to take.

stevekuznetsov · 2016-08-18T18:03:08Z

we didn't have man pages for all our binaries.

Which are missing? We are trying to generate all of the man pages for the CLI from our normal helptext.

tdawson · 2016-08-18T18:42:45Z

We are currently only creating man pages for the main binaries, oadm, oc, and openshift

We don't create any for
atomic-enterprise
dockerregistry
kube-apiserver
kube-controller-manager
kubectl
kubelet
kube-proxy
kubernetes
kube-scheduler
openshift-deploy
openshift-docker-build
openshift-f5-router
openshift-recycle
openshift-router
openshift-sdn-docker-setup.sh
openshift-sdn-ovs
openshift-sti-build
origin
pod

Most of those are just links to openshift, but rpmlint doesn't know that, and some of them act differently depending on how they are called.

stevekuznetsov · 2016-08-18T18:47:25Z

Should be fairly simple to add pages for the links, just change the base name of them. Similarly we could import the kube ones if it's more important than getting fewer nags from rpmlint

smarterclayton · 2016-08-18T22:18:36Z

The -pkgdir statement is wrong - on some platforms it generates content into the user's git repo. I'll open a follow up, but I'll need you @tdawson to review the changes for spec.

ncdc · 2016-08-19T02:22:07Z

hack/common.sh

        eval "platform_goflags=(${!platform_goflags_envvar:-})"

        GOOS=${platform%/*} GOARCH=${platform##*/} go install \
+          -pkgdir ${GOPATH}/src/${OS_GO_PACKAGE} \


@tdawson @smarterclayton @stevekuznetsov FYI this line makes it so the go stdlib as well as all the origin & vendored code have their .a files placed in the root of the origin dir, so you get bufio.a, bytes.a, ...

@damemi in case you were wondering why this was happening

@ncdc Correct. And if we don't have that there, then everything built tries to be put in the go global area (/usr/lib/golang/pkg/linux_386/). This works if you are a privileged user, but doesn't if you are not.
#10027 (comment)

Yeah, that's a problem for non-release builds (putting all the junk into pwd). I think it would be better to only set -pkgdir for release builds.

smarterclayton · 2016-08-19T14:02:10Z

Opened #10533 to track -pkgdir not being the root

tdawson mentioned this pull request Aug 12, 2016

RPM spec builds need to use hack/build-cross.sh #10027

Closed

stevekuznetsov reviewed Aug 12, 2016
View reviewed changes

tdawson force-pushed the 2016-08-spec-use-hack branch from 0edc325 to 7a3a9d6 Compare August 15, 2016 21:01

tdawson mentioned this pull request Aug 18, 2016

Check rpm builds during ci tests #10512

Closed

openshift-bot merged commit 153ad7c into openshift:master Aug 18, 2016

ncdc reviewed Aug 19, 2016
View reviewed changes

Conversation

tdawson commented Aug 12, 2016

Uh oh!

liggitt commented Aug 12, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tdawson commented Aug 13, 2016

Uh oh!

tdawson commented Aug 13, 2016

Uh oh!

openshift-bot commented Aug 15, 2016

Uh oh!

openshift-bot commented Aug 15, 2016

Uh oh!

tdawson commented Aug 17, 2016

Uh oh!

openshift-bot commented Aug 17, 2016

Uh oh!

tdawson commented Aug 18, 2016

Uh oh!

stevekuznetsov commented Aug 18, 2016

Uh oh!

sdodson commented Aug 18, 2016

Uh oh!

stevekuznetsov commented Aug 18, 2016

Uh oh!

openshift-bot commented Aug 18, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-bot commented Aug 18, 2016

Uh oh!

tdawson commented Aug 18, 2016

Uh oh!

stevekuznetsov commented Aug 18, 2016

Uh oh!

tdawson commented Aug 18, 2016

Uh oh!

stevekuznetsov commented Aug 18, 2016

Uh oh!

smarterclayton commented Aug 18, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tdawson Aug 19, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smarterclayton commented Aug 19, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

openshift-bot commented Aug 18, 2016 •

edited

Loading

tdawson Aug 19, 2016 •

edited

Loading