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 1939855: controllers/new: Always pull graph-data images #133

Merged

Conversation

wking
Copy link
Member

@wking wking commented Oct 1, 2021

Official docs recommend:

graphDataImage: registry.example.com/openshift/graph-data:latest

That's a mutable tag, so pull-if-not-present will not fetch new data when graph-data changes. Moving to Always will ensure we have fresh data. Before this commit, the tedious mitigation is to use by-digest pullspecs instead of by-tag pullspecs, and to explicitly bump the digest to push out your fresh graph-data.

This can create some slight overhead when the target container has not changed since the last pull. And in extreme cases where the canonical registry and all configured mirrors are down, it would prevent the operand pod from launching entirely. But I think the appropriate mitigation for that is to configure a working mirror registry. And though the upstream docs are not clear on this point, I see no benefit to contacting a registry about a by-digest pullspec that is already available locally; perhaps the kubelet will not attempt registry access if you use a by-digest pullspec.

Official docs recommend [1]:

  graphDataImage: registry.example.com/openshift/graph-data:latest

That's a mutable tag, so pull-if-not-present will not fetch new data
when graph-data changes.  Moving to 'Always' will ensure we have fresh
data.  Before this commit, the tedious mitigation is to use by-digest
pullspecs instead of by-tag pullspecs, and to explicitly bump the
digest to push out your fresh graph-data.

This can create some slight overhead when the target container has not
changed since the last pull.  And in extreme cases where the canonical
registry and all configured mirrors are down, it would prevent the
operand pod from launching entirely.  But I think the appropriate
mitigation for that is to configure a working mirror registry.  And
though the upstream docs [2] are not clear on this point, I see no
benefit to contacting a registry about a by-digest pullspec that is
already available locally; perhaps the kubelet will not attempt
registry access if you use a by-digest pullspec.

[1]: https://docs.openshift.com/container-platform/4.8/updating/installing-update-service.html#update-service-create-service
[2]: https://kubernetes.io/docs/concepts/containers/images/#image-pull-policy
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 1, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: wking

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 1, 2021
@wking wking changed the title controllers/new: Always pull graph-data images Bug: controllers/new: Always pull graph-data images Oct 1, 2021
@wking wking changed the title Bug: controllers/new: Always pull graph-data images Bug 2009651: controllers/new: Always pull graph-data images Oct 1, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 1, 2021

@wking: This pull request references Bugzilla bug 2009651, 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.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.10.0) matches configured target release for branch (4.10.0)
  • bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

No GitHub users were found matching the public email listed for the QA contact in Bugzilla (jiajliu@redhat.com), skipping review request.

In response to this:

Bug 2009651: controllers/new: Always pull graph-data images

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 openshift-ci bot added bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. labels Oct 1, 2021
@wking wking changed the title Bug 2009651: controllers/new: Always pull graph-data images Bug 1939855: controllers/new: Always pull graph-data images Oct 8, 2021
@openshift-ci openshift-ci bot removed the bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. label Oct 8, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 8, 2021

@wking: This pull request references Bugzilla bug 1939855, 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.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.10.0) matches configured target release for branch (4.10.0)
  • bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

No GitHub users were found matching the public email listed for the QA contact in Bugzilla (jiajliu@redhat.com), skipping review request.

In response to this:

Bug 1939855: controllers/new: Always pull graph-data images

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 openshift-ci bot added the bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. label Oct 8, 2021
@jottofar
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 20, 2021
@openshift-merge-robot openshift-merge-robot merged commit b71ac65 into openshift:master Oct 20, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 20, 2021

@wking: All pull requests linked via external trackers have merged:

Bugzilla bug 1939855 has been moved to the MODIFIED state.

In response to this:

Bug 1939855: controllers/new: Always pull graph-data images

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.

@wking wking deleted the always-pull-graph-data branch October 20, 2021 15:02
@wking
Copy link
Member Author

wking commented Jan 31, 2022

/cherrypick release-4.9

@openshift-cherrypick-robot

@wking: new pull request created: #142

In response to this:

/cherrypick release-4.9

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.

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/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants