fix multus include by darkobas2 · Pull Request #10105 · kubernetes-sigs/kubespray

darkobas2 · 2023-05-18T17:03:59Z

"msg": "Failed to template loop_control.label: 'ansible.utils.unsafe_proxy.AnsibleUnsafeText object' has no attribute 'item'. 'ansible.utils.unsafe_proxy.AnsibleUnsafeText object' has no attribute 'item'", "skip_reason": "Conditional result was False"}

case when multus should NOT be included.

/kind bug

What this PR does / why we need it:

if multus is not enabled, unable to deploy/upgrade.

Which issue(s) this PR fixes:

Special notes for your reviewer:

someone with multus should test this.

Does this PR introduce a user-facing change?:

NONE

linux-foundation-easycla · 2023-05-18T17:04:03Z

The committers listed above are authorized under a signed CLA.

✅ login: darkobas2 (e41b7f3)

k8s-ci-robot · 2023-05-18T17:04:07Z

Welcome @darkobas2!

It looks like this is your first PR to kubernetes-sigs/kubespray 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/kubespray has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2023-05-18T17:04:08Z

Hi @darkobas2. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

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.

oomichi · 2023-05-19T08:07:43Z

/ok-to-test

MrFreezeex · 2023-05-19T11:32:03Z

If the | default('None') is not necessary anymore I would remove it otherwise (or if it still is necessary) lgtm.

darkobas2 · 2023-05-19T12:17:50Z

If the | default('None') is not necessary anymore I would remove it otherwise (or if it still is necessary) lgtm.

well on my setup I did not need it.. but when i uploaded the first patch i noticed the CI job was failing without it.

MrFreezeex · 2023-05-19T13:57:44Z

If the | default('None') is not necessary anymore I would remove it otherwise (or if it still is necessary) lgtm.

well on my setup I did not need it.. but when i uploaded the first patch i noticed the CI job was failing without it.

Could you try again without this line changed? Anyway the CI is a bit flaky these days when there are too much tasks running so you would have to push force to relaunch the CI anyway :x 🙈...

darkobas2 · 2023-05-19T14:07:08Z

i understand your concern... in case multus is needed and these wars are needed playbook should fail... i can look into how to maybe implement this logic where the multus_manifest_x vars are defined... meanwhile the CI should be running

MrFreezeex · 2023-05-19T14:25:54Z

Sorry could you rebase your PR and push force to relaunch the pipeline again 🙏 😬

MrFreezeex · 2023-05-19T17:09:04Z

  delegate_to: "{{ groups['kube_control_plane'][0] }}"
  run_once: true
-  with_items: "{{ multus_manifest_1.results }} + {{ multus_nodes_list|map('extract', hostvars, 'multus_manifest_2')|list|json_query('[].results') }}"
+  with_items: "{{ multus_manifest_1.results + multus_nodes_list|map('extract', hostvars, 'multus_manifest_2.results')|list }}"


So you were indeed right the CI fails with this change :(.

I think something like that should work though:

Suggested change

with_items: "{{ multus_manifest_1.results + multus_nodes_list|map('extract', hostvars, 'multus_manifest_2.results')|list }}"

with_items: "{{ multus_manifest_1.results + (multus_nodes_list|map('extract', hostvars, 'multus_manifest_2.results') | default([]))|list }}"

sorry that didnt work.

"msg": "Failed to template loop_control.label: 'None' has no attribute 'item'. 'None' has no attribute 'item'"

I could not get it to work otherwise, so I added a task after this one to fail if this task resulted with item.name 'None'

i even tried setting

default([{'item': {'name': 'None', 'type': 'None', 'file': 'None'}}]

Hmmm actually I am thinking that the extract might simply not works... Does this works better?

Suggested change

with_items: "{{ multus_manifest_1.results + multus_nodes_list|map('extract', hostvars, 'multus_manifest_2.results')|list }}"

with_items: "{{ multus_manifest_1.results + (multus_nodes_list|map('extract', hostvars, 'multus_manifest_2')|list|json_query('[].results')) }}"

If this works I believe it's the only change that we would really need to have it working consistently...

sorry i cant test locally in the next day or two.... CI is failing too.. will push again later to retry

No worries, thanks for all your OSS works ❤️

`` "msg": "Failed to template loop_control.label: 'ansible.utils.unsafe_proxy.AnsibleUnsafeText object' has no attribute 'item'. 'ansible.utils.unsafe_proxy.AnsibleUnsafeText object' has no attribute 'item'", "skip_reason": "Conditional result was False"} `` fixes case when multus should NOT be included.

floryut

@darkobas2 Thank you for the PR 👍

MrFreezeex

Thanks for the contribution and bearing with me and all the research that went into this! ❤️
/lgtm

k8s-ci-robot · 2023-05-23T08:10:15Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: darkobas2, floryut, MrFreezeex

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [floryut]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

`` "msg": "Failed to template loop_control.label: 'ansible.utils.unsafe_proxy.AnsibleUnsafeText object' has no attribute 'item'. 'ansible.utils.unsafe_proxy.AnsibleUnsafeText object' has no attribute 'item'", "skip_reason": "Conditional result was False"} `` fixes case when multus should NOT be included.

k8s-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label May 18, 2023

k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label May 18, 2023

k8s-ci-robot requested review from liupeng0518 and yankay May 18, 2023 17:04

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label May 18, 2023

k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels May 18, 2023

MrFreezeex suggested changes May 18, 2023

View reviewed changes

Comment thread roles/kubernetes-apps/network_plugin/meta/main.yml Outdated

Comment thread roles/network_plugin/meta/main.yml Outdated

Comment thread roles/kubernetes-apps/network_plugin/multus/tasks/main.yml Outdated

MrFreezeex reviewed May 19, 2023

View reviewed changes

yankay mentioned this pull request May 23, 2023

Release Proposal v2.22 #10075

Closed

floryut approved these changes May 23, 2023

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 23, 2023

MrFreezeex approved these changes May 23, 2023

View reviewed changes

k8s-ci-robot assigned MrFreezeex May 23, 2023

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 23, 2023

k8s-ci-robot merged commit 4014a1c into kubernetes-sigs:master May 23, 2023

blackluck mentioned this pull request Jul 20, 2023

multus - mixed type of container engine support commit fails #10305

Closed

	with_items: "{{ multus_manifest_1.results + multus_nodes_list\|map('extract', hostvars, 'multus_manifest_2.results')\|list }}"
	with_items: "{{ multus_manifest_1.results + (multus_nodes_list\|map('extract', hostvars, 'multus_manifest_2.results') \| default([]))\|list }}"

Conversation

darkobas2 commented May 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

linux-foundation-easycla Bot commented May 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented May 18, 2023

Uh oh!

k8s-ci-robot commented May 18, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

oomichi commented May 19, 2023

Uh oh!

MrFreezeex commented May 19, 2023

Uh oh!

darkobas2 commented May 19, 2023

Uh oh!

MrFreezeex commented May 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

darkobas2 commented May 19, 2023

Uh oh!

MrFreezeex commented May 19, 2023

Uh oh!

MrFreezeex May 19, 2023 • 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!

darkobas2 May 19, 2023

Choose a reason for hiding this comment

Uh oh!

darkobas May 19, 2023

Choose a reason for hiding this comment

Uh oh!

MrFreezeex May 21, 2023 • 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!

darkobas2 May 22, 2023

Choose a reason for hiding this comment

Uh oh!

MrFreezeex May 22, 2023

Choose a reason for hiding this comment

Uh oh!

floryut left a comment

Choose a reason for hiding this comment

Uh oh!

MrFreezeex left a comment • 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!

k8s-ci-robot commented May 23, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

darkobas2 commented May 18, 2023 •

edited

Loading

linux-foundation-easycla Bot commented May 18, 2023 •

edited

Loading

MrFreezeex commented May 19, 2023 •

edited

Loading

MrFreezeex May 19, 2023 •

edited

Loading

MrFreezeex May 21, 2023 •

edited

Loading

MrFreezeex left a comment •

edited

Loading