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

fix: join node's installCNI step failed. #193

Merged
merged 1 commit into from
Sep 6, 2022

Conversation

lixd
Copy link
Contributor

@lixd lixd commented Sep 5, 2022

Signed-off-by: lixd xueduan.li@gmail.com

What type of PR is this?

/kind bug

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #192

Special notes for reviewers:

some step need latest step,s response ,so there is a check

if len(operation.Status.Conditions[i-1].Status) < 1 {
    return errors.New("unexpected error, steps node field must be valid")
 }

and will generte Status.Conditions[i-1].Status by cond.Status := make([]v1.StepStatus, len(step.Nodes))
so,if step.Nodes == nil, len(cond.Status) is 0,and will retrun error on if len(operation.Status.Conditions[i-1].Status) < 1 check.
installCNI setp caller like this:

steps, err = cn.InitStepper(&stepper.Cluster.CNI, &stepper.Cluster.Networking).InstallSteps(patchNodes, nil, true)

second args is nil.

installCNI setp like this:

func (stepper *CNIInfo) InstallSteps(allNodes, firstMasterNodes []v1.StepNode, onlyLoad bool) ([]v1.Step, error) {
        var steps []v1.Step
        bytes, err := json.Marshal(stepper)
        if err != nil {
                return nil, err
        }
        if stepper.CNI.Offline && stepper.CNI.LocalRegistry == "" {
                steps = []v1.Step{
                        {
                                ID:         strutil.GetUUID(),
                                Name:       "cniImageLoader",
                        },
                }
        // only load images for some special scenarios, such as JoinNode
        if onlyLoad {
                return steps, nil
        }
        return append(steps, v1.Step{
                ID:         strutil.GetUUID(),
                Name:       "installCNI",
                Timeout:    metav1.Duration{Duration: 1 * time.Minute},
                ErrIgnore:  false,
                RetryTimes: 1,
                Nodes:      firstMasterNodes,
                Commands: []v1.Command{
        }), nil

In installCNI step,used second args as step.Nodes,so this will omit error.

there is a onlyload check but has a mistake,if don't in if stepper.CNI.Offline && stepper.CNI.LocalRegistry == "" { it doesn't work, this pr fix it.

and if click retry,will run from next step,so it success.

Does this PR introduced a user-facing change?

None

Additional documentation, usage docs, etc.:


Signed-off-by: lixd <xueduan.li@gmail.com>
@kubeclipper-bot kubeclipper-bot added release-note-none kind/bug Categorizes issue or PR as related to a bug. dco-signoff: yes size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Sep 5, 2022
@codecov-commenter
Copy link

Codecov Report

Merging #193 (91d172c) into master (b87e163) will not change coverage.
The diff coverage is 0.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #193   +/-   ##
=======================================
  Coverage   13.00%   13.00%           
=======================================
  Files         105      105           
  Lines       16026    16026           
=======================================
  Hits         2084     2084           
  Misses      13691    13691           
  Partials      251      251           
Impacted Files Coverage Δ
pkg/scheme/core/v1/k8s/kubeadm_step.go 0.00% <0.00%> (ø)

@lixd
Copy link
Contributor Author

lixd commented Sep 5, 2022

/cc @qinyer

@qinyer
Copy link
Contributor

qinyer commented Sep 5, 2022

/lgtm

@x893675
Copy link
Collaborator

x893675 commented Sep 5, 2022

/approve

@kubeclipper-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lixd, x893675

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

@kubeclipper-bot kubeclipper-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 5, 2022
@qinyer
Copy link
Contributor

qinyer commented Sep 6, 2022

/lgtm

@kubeclipper-bot kubeclipper-bot added the lgtm Indicates that a PR is ready to be merged. label Sep 6, 2022
@kubeclipper-bot
Copy link
Collaborator

LGTM label has been added.

Git tree hash: 3e8e16c6f1fe7b513ff614940e2a0010bc58a836

@kubeclipper-bot kubeclipper-bot merged commit a15ec8e into kubeclipper:master Sep 6, 2022
@lixd lixd deleted the fix_join_node branch October 25, 2022 05:42
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. dco-signoff: yes kind/bug Categorizes issue or PR as related to a bug. lgtm Indicates that a PR is ready to be merged. release-note-none size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cluster join node failed,retry success.
5 participants