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

Bug 1771335: Verify pullsecret builds #24148

Merged

Conversation

adambkaplan
Copy link
Contributor

No description provided.

@openshift-ci-robot openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. labels Nov 13, 2019
@openshift-ci-robot
Copy link

@adambkaplan: This pull request references Bugzilla bug 1771335, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

[WIP] Bug 1771335: Verify pullsecret builds

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 13, 2019
@adambkaplan
Copy link
Contributor Author

fyi @gabemontero @nalind - this should verify that we're using a pull secret correctly in builds. Right now something is preventing this from working correctly.

@adambkaplan adambkaplan changed the title [WIP] Bug 1771335: Verify pullsecret builds Bug 1771335: Verify pullsecret builds Nov 14, 2019
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 14, 2019
@adambkaplan
Copy link
Contributor Author

@gabemontero squashed this, gcp-builds should pass with your fix.

@gabemontero
Copy link
Contributor

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 14, 2019
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

1 similar comment
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@gabemontero
Copy link
Contributor

google iam policy problem with e2e-gcp-builds

@gabemontero
Copy link
Contributor

more ci install flakes with e2e-gcp-builds

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

9 similar comments
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@gabemontero
Copy link
Contributor

still install flakes on e2e-gcp-builds

@gabemontero
Copy link
Contributor

ok we got a clean install but your test case still failed @adambkaplan

apparently my manual testing using my tbr cred and the new-app invocation from the bug is not quite the same as your new test case

I'll start running your new test case manually with some additional debug and see what is going on

@gabemontero
Copy link
Contributor

/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 15, 2019
Copy link
Contributor

@gabemontero gabemontero left a comment

Choose a reason for hiding this comment

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

the linked secret test case worked for me locally ... see my comment below about the yaml correction you need to make the other one work @adambkaplan

from:
kind: DockerImage
name: registry.redhat.io/rhscl/nodejs-10-rhel7:latest
pullSecret:
Copy link
Contributor

Choose a reason for hiding this comment

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

@adambkaplan looks like the indentation on you yaml is resulting in this pull secret not getting pulled in

See that it is missing when I use it as is to create the BC:

gmontero ~/QE_bzs/img-src-auth $ cat pullsecret-nodejs.yaml 
kind: BuildConfig
apiVersion: v1
metadata:
  name: pullsecret-nodejs
spec:
  source:
    git:
      uri: "https://github.com/sclorg/nodejs-ex.git"
  strategy:
    type: Source
    sourceStrategy:
      from:
        kind: DockerImage
        name: registry.redhat.io/rhscl/nodejs-10-rhel7:latest
    pullSecret:
      name: local-ps
gmontero ~/QE_bzs/img-src-auth $ oc get bc -o yaml
apiVersion: v1
items:
- apiVersion: build.openshift.io/v1
  kind: BuildConfig
  metadata:
    creationTimestamp: "2019-11-15T18:35:13Z"
    name: pullsecret-nodejs
    namespace: ggmtest
    resourceVersion: "49618"
    selfLink: /apis/build.openshift.io/v1/namespaces/ggmtest/buildconfigs/pullsecret-nodejs
    uid: e4c6808a-da49-4a11-b9e1-ca736e533014
  spec:
    failedBuildsHistoryLimit: 5
    nodeSelector: null
    output: {}
    postCommit: {}
    resources: {}
    runPolicy: Serial
    source:
      git:
        uri: https://github.com/sclorg/nodejs-ex.git
      type: Git
    strategy:
      sourceStrategy:
        from:
          kind: DockerImage
          name: registry.redhat.io/rhscl/nodejs-10-rhel7:latest
      type: Source
    successfulBuildsHistoryLimit: 5
    triggers: []
  status:
    lastVersion: 1
kind: List
metadata:
  resourceVersion: ""
  selfLink: ""
gmontero ~/QE_bzs/img-src-auth $

when I indent pullSecret in by 2 spaces, it is incorporated:

gmontero ~/QE_bzs/img-src-auth $ oc get bc -o yaml
apiVersion: v1
items:
- apiVersion: build.openshift.io/v1
  kind: BuildConfig
  metadata:
    creationTimestamp: "2019-11-15T18:45:03Z"
    name: pullsecret-nodejs
    namespace: ggmtest
    resourceVersion: "52110"
    selfLink: /apis/build.openshift.io/v1/namespaces/ggmtest/buildconfigs/pullsecret-nodejs
    uid: e56ff415-94da-4da0-b20b-a53f2e7c49dd
  spec:
    failedBuildsHistoryLimit: 5
    nodeSelector: null
    output: {}
    postCommit: {}
    resources: {}
    runPolicy: Serial
    source:
      git:
        uri: https://github.com/sclorg/nodejs-ex.git
      type: Git
    strategy:
      sourceStrategy:
        from:
          kind: DockerImage
          name: registry.redhat.io/rhscl/nodejs-10-rhel7:latest
        pullSecret:
          name: local-ps
      type: Source
    successfulBuildsHistoryLimit: 5
    triggers: []
  status:
    lastVersion: 0
kind: List
metadata:
  resourceVersion: ""
  selfLink: ""
gmontero ~/QE_bzs/img-src-auth $ 

and the build ultimatey works with my changes to openshift/builder

please update this and we'll try the e2e in this PR again

Copy link
Contributor

Choose a reason for hiding this comment

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

the corrected pullsecret-nodejs yaml:

gmontero ~/QE_bzs/img-src-auth $ cat pullsecret-nodejs.yaml
kind: BuildConfig
apiVersion: v1
metadata:
  name: pullsecret-nodejs
spec:
  source:
    git:
      uri: "https://github.com/sclorg/nodejs-ex.git"
  strategy:
    type: Source
    sourceStrategy:
      from:
        kind: DockerImage
        name: registry.redhat.io/rhscl/nodejs-10-rhel7:latest
      pullSecret:
        name: local-ps
gmontero ~/QE_bzs/img-src-auth $ 

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gabemontero gotta love YAML. Fixed in the next push.

Copy link
Contributor

Choose a reason for hiding this comment

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

yes you do :-)

@gabemontero
Copy link
Contributor

/lgtm cancel

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Nov 15, 2019
@gabemontero
Copy link
Contributor

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 15, 2019
Use pull secret for registry.redhat.io to verify that builds can pull
from an external registry with a pull secret.
@gabemontero
Copy link
Contributor

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 18, 2019
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adambkaplan, gabemontero

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@gabemontero
Copy link
Contributor

prometheus failures in e2e-gcp

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@gabemontero
Copy link
Contributor

e2e-gcp-builds is passing @adambkaplan

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@gabemontero
Copy link
Contributor

install flake e2e-gcp

/retest

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit b422434 into openshift:master Nov 19, 2019
@openshift-ci-robot
Copy link

@adambkaplan: All pull requests linked via external trackers have merged. Bugzilla bug 1771335 has been moved to the MODIFIED state.

In response to this:

Bug 1771335: Verify pullsecret builds

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@gabemontero
Copy link
Contributor

@adambkaplan do we want to cherrypick this test back to 4.2 / 4.1 since we are backporting the CVE fix to not always setting skip tls verify to true to those destinations ?

deads2k added a commit to deads2k/origin that referenced this pull request Dec 13, 2019
…llsecret-builds"

This reverts commit b422434, reversing
changes made to 5c93a45.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants